Skip to content

Conversation

@turbomam
Copy link
Member

  • Add storage_units annotation to exp_pipe slot (unitless: "1")
  • Add storage_units annotation to occup_density_samp slot ("1/[sft_i]")
  • Add "1/[sft_i]" (per square foot) unit to UnitEnum in attribute_values.yaml
  • Make biomaterial_purity slot abstract (requires migrator for dna_absorb1/dna_absorb2)
  • Make concentration slot abstract with comment documenting union of child units (requires migrator for source_concentration/final_concentration)
  • Update test_units_alignment.py to enforce that abstract QuantityValue slots must NOT have storage_units or units_alignment_excuse annotations

Addresses #2687

🤖 Generated with Claude Code

…al_purity and concentration abstract

- Add storage_units annotation to exp_pipe slot (unitless: "1")
- Add storage_units annotation to occup_density_samp slot ("1/[sft_i]")
- Add "1/[sft_i]" (per square foot) unit to UnitEnum in attribute_values.yaml
- Make biomaterial_purity slot abstract (requires migrator for dna_absorb1/dna_absorb2)
- Make concentration slot abstract with comment documenting union of child units (requires migrator for source_concentration/final_concentration)
- Update test_units_alignment.py to enforce that abstract QuantityValue slots must NOT have storage_units or units_alignment_excuse annotations

Addresses #2687

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@turbomam turbomam linked an issue Oct 20, 2025 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Oct 20, 2025

PR Preview Action v1.6.2

🚀 View preview at
https://microbiomedata.github.io/nmdc-schema/pr-preview/pr-2690/

Built to branch gh-pages at 2025-10-21 01:50 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@turbomam
Copy link
Member Author

turbomam commented Oct 21, 2025

@pkalita-lbl @eecavanna I'm going to need some help with this PR

I can break it up into multiple single topic PRs if necessary

  • since it puts new constraints on the storage_units that some slots can use, I assume it will need a migrator
  • it didn't seem like tests/test_nmdc_schema_validation_plugin.py was catching newly invalid has_unit values in src/data/valid, so i switched the test harness from unittest to pytest. That required configuring pytest to only run tests in tests/ and not the migration tests that connect to MongoDB, etc. Does that look OK to you?

@pkalita-lbl
Copy link
Collaborator

I definitely support switching from unittest to pytest (although I haven't looked at the actual changes yet). Do you understand why changing the test runner changed the outcome of the tests? That concerns me.

@eecavanna
Copy link
Collaborator

not the migration tests that connect to MongoDB, etc.

I'm not aware of any migration-related tests that connect to MongoDB.

Here's a list of the migration-related tests I want to continue to be run as part of the automated test suite:

  • doctests within all modules within the nmdc_schema/migrators/ directory
  • tests in test_*.py modules within the nmdc_schema/migrators/adapters/ directory


test-python:
$(RUN) python -m unittest discover
$(RUN) pytest
Copy link
Member Author

Choose a reason for hiding this comment

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

make sure its a dev dependency

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.

non-storage-units solutions for slots with excuses

3 participants