Skip to content

Add check to make sure all reserved keywords are attributes and not properties - #1490

Merged
sarahyurick merged 8 commits into
NVIDIA-NeMo:mainfrom
omkar-334:fix-property
Feb 12, 2026
Merged

Add check to make sure all reserved keywords are attributes and not properties#1490
sarahyurick merged 8 commits into
NVIDIA-NeMo:mainfrom
omkar-334:fix-property

Conversation

@omkar-334

Copy link
Copy Markdown
Contributor

closes #1481

@copy-pr-bot

copy-pr-bot Bot commented Feb 11, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@greptile-apps

greptile-apps Bot commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

This PR adds validation to ensure that the reserved keywords (name, resources, batch_size) are not defined as @property decorators in ProcessingStage subclasses. This is necessary because the with_() method (lines 259-284 in base.py) directly assigns to these attributes to override configuration, which would fail if they were read-only properties.

Changes:

  • Added validation loop in __init_subclass__ (base.py:115-122) that raises TypeError if any of the three reserved keywords are defined as properties
  • Fixed existing violations by converting @property def name() to plain class attributes in BaseSyntheticStage, DiverseQAPostProcessingStage, and KnowledgeListPostProcessingStage
  • Added three new tests to verify the validation works for all three reserved keywords

The implementation is clean, well-tested, and includes helpful error messages that guide developers to use plain attributes or dataclass fields instead of properties.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is straightforward and addresses a real issue where @property decorators prevented the with_() method from working correctly. The changes include comprehensive test coverage, fix all existing violations in the codebase, and use a clear error message to guide developers
  • No files require special attention

Important Files Changed

Filename Overview
nemo_curator/stages/base.py Added validation in __init_subclass__ to prevent reserved keywords (name, resources, batch_size) from being defined as @property decorators, ensuring with_() can override them
nemo_curator/stages/synthetic/nemotron_cc/base.py Converted name from @property to plain class attribute to comply with new validation requirements
nemo_curator/stages/synthetic/nemotron_cc/nemotron_cc.py Converted name from @property to plain class attribute in DiverseQAPostProcessingStage and KnowledgeListPostProcessingStage
tests/stages/common/test_base.py Added comprehensive test coverage for the new validation that prevents name, resources, and batch_size from being defined as @property decorators

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Comment thread nemo_curator/stages/base.py

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

ayushdg and others added 5 commits February 12, 2026 01:15
Signed-off-by: Ayush Dattagupta <ayushdg95@gmail.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>
NVIDIA-NeMo#1471)

* Update transformers dependency to exact version 4.55.2 in pyproject.toml and uv.lock to prevent import failures in Cosmos Embed. Downgrade tokenizers version to 0.21.4 for compatibility.

Signed-off-by: Abhinav Garg <abhinavg@stanford.edu>

* Update transformers dependency in pyproject.toml and uv.lock to allow versions up to 4.55.2, ensuring compatibility with Cosmos Embed imports.

Signed-off-by: Abhinav Garg <abhinavg@stanford.edu>

---------

Signed-off-by: Abhinav Garg <abhinavg@stanford.edu>
Co-authored-by: Ayush Dattagupta <ayushdg95@gmail.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>
* docs: vdr feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/admin/installation.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/admin/installation.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/admin/installation.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/curate-text/process-data/quality-assessment/distributed-classifier.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* re order sidebar

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* release notes draft

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* remove more internvid content

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* release note fix

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/curate-video/tutorials/split-dedup.md

Signed-off-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>

---------

Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>
@omkar-334

Copy link
Copy Markdown
Contributor Author

The commits got messed up while rebasing and signing the commits I guess..

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Comment thread tests/stages/common/test_base.py Outdated
def process(self, task: MockTask) -> MockTask:
return task

def test_plain_attribute_override_allowed(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this may not be needed and should already be covered by TestProcessingStageWith which uses ConcreteProcessingStage. What do you think @omkar-334 ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right... TestProcessingStageWith already tests for attribute overrides. Thanks for this!. Ive removed it now

Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@sarahyurick

Copy link
Copy Markdown
Contributor

/ok to test ba29581

@sarahyurick sarahyurick left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @omkar-334 for the quick response!

@sarahyurick
sarahyurick merged commit 2762f6e into NVIDIA-NeMo:main Feb 12, 2026
50 checks passed
omkar-334 added a commit to omkar-334/Curator that referenced this pull request Feb 21, 2026
…roperties (NVIDIA-NeMo#1490)

* Fix: fasttext predict call for numpy>2 (NVIDIA-NeMo#1482)

Signed-off-by: Ayush Dattagupta <ayushdg95@gmail.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>

* Update transformers dependency to exact version 4.55.2 in pyproject.t… (NVIDIA-NeMo#1471)

* Update transformers dependency to exact version 4.55.2 in pyproject.toml and uv.lock to prevent import failures in Cosmos Embed. Downgrade tokenizers version to 0.21.4 for compatibility.

Signed-off-by: Abhinav Garg <abhinavg@stanford.edu>

* Update transformers dependency in pyproject.toml and uv.lock to allow versions up to 4.55.2, ensuring compatibility with Cosmos Embed imports.

Signed-off-by: Abhinav Garg <abhinavg@stanford.edu>

---------

Signed-off-by: Abhinav Garg <abhinavg@stanford.edu>
Co-authored-by: Ayush Dattagupta <ayushdg95@gmail.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>

* remove property and add check

Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>

* add tests

Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>

* docs: vdr feedback (NVIDIA-NeMo#1477)

* docs: vdr feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/admin/installation.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/admin/installation.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/admin/installation.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/curate-text/process-data/quality-assessment/distributed-classifier.md

Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>

* feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* re order sidebar

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* release notes draft

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* feedback

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* remove more internvid content

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* release note fix

Signed-off-by: Lawrence Lane <llane@nvidia.com>

* Update docs/curate-video/tutorials/split-dedup.md

Signed-off-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>

---------

Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>

* remove override test

Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>

---------

Signed-off-by: Ayush Dattagupta <ayushdg95@gmail.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>
Signed-off-by: Abhinav Garg <abhinavg@stanford.edu>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Co-authored-by: Ayush Dattagupta <ayushdg95@gmail.com>
Co-authored-by: Abhinav Garg <abhinavg@stanford.edu>
Co-authored-by: Lawrence Lane <llane@nvidia.com>
Co-authored-by: Sarah Yurick <53962159+sarahyurick@users.noreply.github.com>
Signed-off-by: Omkar Kabde <omkarkabde@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make sure all reserved keywords are attributes and not properties [name / resources / batch_size]

5 participants