Skip to content

Conversation

@Neo3333
Copy link
Contributor

@Neo3333 Neo3333 commented Oct 16, 2025

This change is basically mlcroissant library update to support new spec change described in pull#951.

@github-actions
Copy link

github-actions bot commented Oct 16, 2025

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@Neo3333 Neo3333 force-pushed the issue-770 branch 4 times, most recently from a3b9b83 to 13e7261 Compare October 23, 2025 07:24
@Neo3333 Neo3333 marked this pull request as ready for review October 23, 2025 15:30
@Neo3333 Neo3333 requested a review from a team as a code owner October 23, 2025 15:30
@Neo3333 Neo3333 requested a review from ccl-core October 23, 2025 15:37
)
value = apply_transforms_fn(row[column], field=field)
has_source = bool(field.source)
if has_source:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should assert that source and value are mutually exclusive, and than one of them must be specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, see comment below.

for node in entry_nodes:
if isinstance(node, RecordSet) and not node.data:
for field in node.fields:
if not field.source:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is the right place to check if field.source exists that field.value must not be present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added logic in field.py to make sure these two members are mutually exclusive. In graph.py added logic to make sure exactly one of these two members are set.

Copy link
Contributor

@benjelloun benjelloun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Neo3333 Neo3333 merged commit 18dfce8 into mlcommons:main Nov 5, 2025
12 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants