Skip to content

PNE-6482 Support default_labels in MaterialRun constructor #989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 27, 2025

Conversation

jamie-heller
Copy link
Contributor

@jamie-heller jamie-heller commented Mar 25, 2025

Citrine Python PR

PNE-6482

Support default_labels in MaterialRun constructor

This PR adds support for a new optional default_labels: List[str] parameter in the MaterialRun constructor.

Default labels are used to:

  • Populate labels on the ingredient run, if none are explicitly specified, when the material run is later used as an ingredient
  • Marking the material run as a potential replacement ingredient for a particular label when generating new candidates using a design space. Note that during design, default labels are only applicable if the material run has no associated ingredient run within the training set in question.

PR Type:

  • Breaking change (fix or feature that would cause existing functionality to change)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Maintenance (non-breaking change to assist developers)

Adherence to team decisions

  • I have added tests for 100% coverage
  • I have written Numpy-style docstrings for every method and class.
  • I have communicated the downstream consequences of the PR to others.
  • I have bumped the version in __version__.py

@jamie-heller jamie-heller force-pushed the task/pne-6482-default-labels branch 6 times, most recently from cf8dea5 to 3c33bf1 Compare March 25, 2025 17:43
@jamie-heller jamie-heller changed the title DRAFT PNE-6482 Support default_labels in MaterialRun constructor PNE-6482 Support default_labels in MaterialRun constructor Mar 25, 2025
@jamie-heller jamie-heller force-pushed the task/pne-6482-default-labels branch from 3c33bf1 to 4617536 Compare March 25, 2025 18:18
@jamie-heller jamie-heller marked this pull request as ready for review March 25, 2025 20:27
@jamie-heller jamie-heller requested a review from a team as a code owner March 25, 2025 20:27
@kroenlein
Copy link
Collaborator

Why is this being update here and not in https://github.com/CitrineInformatics/gemd-python/blob/main/gemd/entity/template/material_template.py? Nominally, that's where the data structure is inherited from.

@jamie-heller
Copy link
Contributor Author

Why is this being update here and not in https://github.com/CitrineInformatics/gemd-python/blob/main/gemd/entity/template/material_template.py? Nominally, that's where the data structure is inherited from.

This seemed to be the most directly equivalent to the logic that was added in https://github.com/CitrineInformatics/platform-backend/pull/5021 to support the design-over-untested-ingredients functionality via UI ingest -- adding the generated tags to the material run.

Since we don't add tags to the material template in that workflow I think it probably makes sense to limit the scope of this change as well, but I'm open to reconsidering.

kroenlein
kroenlein previously approved these changes Mar 26, 2025
Copy link
Collaborator

@kroenlein kroenlein left a comment

Choose a reason for hiding this comment

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

This is fit for purpose. Some stylistic feedback to be provided off-line.

Copy link
Collaborator

@kroenlein kroenlein left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the additional tests.

@jamie-heller jamie-heller merged commit f98c68c into main Mar 27, 2025
17 checks passed
@jamie-heller jamie-heller deleted the task/pne-6482-default-labels branch March 27, 2025 17:56
jamie-heller added a commit that referenced this pull request Mar 31, 2025
This update is analagous to the one in #989

The associated utility function and test have been moved to a shared module,
and the documentation has been updated
jamie-heller added a commit that referenced this pull request Mar 31, 2025
This update is analagous to the one in #989

The associated utility function and test have been moved to a shared module,
and the documentation has been updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants