Typecode fixes on IDs for `ChromatographyConfiguration`, `StorageProcess`, and `MassSpectrometryConfiguration` class ids #225

kheal · 2024-07-16T20:02:21Z

PR Information

What type of PR is this? (check all applicable)

Bug Fix
Documentation

Description

This PR aims to

Shorten id typecodes to adhere to nmdc standards for two berkeley-schema specific classes (aka, no data in mongo for these).
- ChromatographyConfiguration typecode changed from chrocon to chrcon
- StorageProcess typecode changed from storpro to storpr
All tests and references to these old typecodes
Update CONTRIBUTING guide to reflect the typecode guidelines
Remove extraneous parentheses for ids on definition of MassSpectrometryConfiguration and ChromatographyConfiguration.

Related Issues

Addresses the schema-relevant portion of: berkeley-schema-fy24: id slot_usage resulting in badly minted IDs nmdc-schema#2122 . The other aspect of that issue is documented here: berkeley: Update minter to use first typecode when pattern contains multiple typecodes nmdc-runtime#592

Did you add/update any tests?

Yes I updated the example data that is tested

Could this schema change make it so any valid data becomes invalid?

Yes (A migrator is required)
there are no existing data in mongo that are part of these classes (we discovered these issues when trying to mint IDs in order to load these classes). @eecavanna are you OK with no migrator for this change? I understand that this is not best practice, but making a migrator for theoretical records with ids that could not have been minted seems a bit....unnecessary.

If you answered "Yes", does this PR branch include that migrator?

No, but we've decided it it unnecessary, see conversation below

Does this PR have any downstream implications?

No

github-actions · 2024-07-16T20:04:33Z

PR Preview Action v1.4.7
🚀 Deployed preview to https://microbiomedata.github.io/berkeley-schema-fy24/pr-preview/pr-225/
on branch `gh-pages` at 2024-07-17 23:14 UTC

eecavanna · 2024-07-16T21:44:36Z

Hi @kheal, thanks for including all that info and for tagging me in the migration part.

I have a question:

Were those fancy patterns present in the legacy schema? If not, then no migrator is necessary, from my perspective. Elaboration is below.

In the Berkeley schema (at this stage, where we haven't yet merged it back into the upstream repo), I consider all schema changes to be part of a single set of "legacy-to-Berkeley" schema changes that get applied at once. So, if those fancy patterns did not exist in the legacy schema, exist in the current Berkeley schema, and will not exist in the Berkeley schema after you merge this PR in, I will effectively act as though they never existed.

Out sick this afternoon and typing this from my phone. Hope that helps a bit.

turbomam · 2024-07-16T21:44:50Z

CONTRIBUTING.md

@@ -109,6 +109,9 @@ Core developers should read the material on the [LinkML site](https://linkml.io/
    - Follow the naming conventions of the parent class
    - Descriptions of child classes may reference parent classes in a genus-differentia definition structure (e.g. "A workflow execution activity that...")
    - Inheritance should be monotonic: `slot_usage` should refine rather than override
+- ID patterning and checks
+    - ID patterns for new classes should follow conventions found [here](https://microbiomedata.github.io/nmdc-schema/identifiers/)
+    - Class-linking slots (i.e. `has_input`) should have slot_usage declared that limit range to only instances of the expecting linked class (i.e. `syntax: "{id_nmdc_prefix}:chrcon-{id_shoulder}-{id_blade}$"`)


You're talking about the structured_patterns here, right? The range is limited by the range metaslot, and not anything else. This is also intended as a short term fix, just until the LinkML developers come up with a more general solution that doesn't require us to make repetitive assertions.

Yes, this is in reference to the structured_pattern use. I understand that its a temporary fix, but it's been a big pain for the re-IDing team so I wanted to document it somewhere until linkML can verify ranges.

I will update language to be more explicit.

turbomam · 2024-07-16T21:45:23Z

thanks, good issues and good PR

kheal · 2024-07-16T21:50:54Z

@eecavanna

Re:

Were those fancy patterns present in the legacy schema? If not, then no migrator is necessary, from my perspective. Elaboration is below.

No they were not. @turbomam nicely documented that for us here: microbiomedata#2122 (comment) .

I agree re: no migrator, thanks!

CONTRIBUTING.md

Co-authored-by: eecavanna <134325062+eecavanna@users.noreply.github.com>

eecavanna · 2024-07-18T00:42:12Z

Remove extraneous parentheses for ids on...

FYI: The Minter in the Berkeley environment has been updated to (a) tolerate one layer of extraneous parentheses and (b) to use the first typecode — foo — in a (foo|bar|baz) list.

mslarae13 · 2024-07-19T22:25:37Z

approve from non-technical review.
Does this relate to microbiomedata#2125 at all?

kheal · 2024-07-22T16:55:50Z

@mslarae13

approve from non-technical review. Does this relate to microbiomedata#2125 at all?

Yes it is related in that it is dealing with typecodes for NMDC ids, but it does not specifically address changes in the schema related to supporting legacy typecodes.

kheal added 4 commits July 16, 2024 12:43

Remove extraneous parentheses on ids

47e144a

Shorten typecodes to adhere to standards

d47712d

Add to Modeling Best Practices for id practices

ff7d336

Fix typecodes on example data and class-linking slots

c48673b

kheal changed the title ~~2122 typecode fix~~ Typecode fixes on IDs for ChromatographyConfiguration, StorageProcess, and MassSpectrometryConfiguration class ids Jul 16, 2024

kheal marked this pull request as ready for review July 16, 2024 21:36

kheal requested a review from mslarae13 July 16, 2024 21:36

kheal self-assigned this Jul 16, 2024

kheal requested review from aclum and turbomam July 16, 2024 21:37

Elaborate on CONTRIBUTING guide regarding ids

27ce828

turbomam reviewed Jul 16, 2024

View reviewed changes

eecavanna approved these changes Jul 16, 2024

View reviewed changes

aclum approved these changes Jul 16, 2024

View reviewed changes

Elaborate on ID patterning and checks

4c2d3b6

kheal mentioned this pull request Jul 17, 2024

berkeley: Update minter to use first typecode when pattern contains multiple typecodes microbiomedata/nmdc-runtime#597

Merged

8 tasks

kheal added 2 commits July 17, 2024 09:25

Add note regarding multiple typecodes

0455c02

Add note regarding multiple typecodes

16a3bad

kheal requested review from naglepuff, SamuelPurvine and sujaypatil96 July 17, 2024 20:21

mslarae13 approved these changes Jul 17, 2024

View reviewed changes

kheal requested a review from mslarae13 July 17, 2024 20:23

eecavanna reviewed Jul 17, 2024

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

eecavanna reviewed Jul 17, 2024

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

Update CONTRIBUTING.md

6b31abb

Co-authored-by: eecavanna <134325062+eecavanna@users.noreply.github.com>

Update CONTRIBUTING.md

44ae17e

Co-authored-by: eecavanna <134325062+eecavanna@users.noreply.github.com>

eecavanna mentioned this pull request Jul 18, 2024

berkeley-schema-fy24: id slot_usage resulting in badly minted IDs microbiomedata/nmdc-schema#2122

Closed

2 tasks

kheal merged commit a74d8d6 into main Jul 24, 2024
3 checks passed

kheal deleted the 2122_typecode_fix branch August 2, 2024 19:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typecode fixes on IDs for `ChromatographyConfiguration`, `StorageProcess`, and `MassSpectrometryConfiguration` class ids #225

Typecode fixes on IDs for `ChromatographyConfiguration`, `StorageProcess`, and `MassSpectrometryConfiguration` class ids #225

kheal commented Jul 16, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024 •

edited

Loading

eecavanna commented Jul 16, 2024 •

edited

Loading

turbomam Jul 16, 2024

kheal Jul 16, 2024

turbomam commented Jul 16, 2024 •

edited

Loading

kheal commented Jul 16, 2024

eecavanna commented Jul 18, 2024 •

edited

Loading

mslarae13 commented Jul 19, 2024

kheal commented Jul 22, 2024 •

edited

Loading

Typecode fixes on IDs for ChromatographyConfiguration, StorageProcess, and MassSpectrometryConfiguration class ids #225

Typecode fixes on IDs for ChromatographyConfiguration, StorageProcess, and MassSpectrometryConfiguration class ids #225

Conversation

kheal commented Jul 16, 2024 • edited Loading

PR Information

What type of PR is this? (check all applicable)

Description

Related Issues

Did you add/update any tests?

Could this schema change make it so any valid data becomes invalid?

If you answered "Yes", does this PR branch include that migrator?

Does this PR have any downstream implications?

github-actions bot commented Jul 16, 2024 • edited Loading

eecavanna commented Jul 16, 2024 • edited Loading

turbomam Jul 16, 2024

Choose a reason for hiding this comment

kheal Jul 16, 2024

Choose a reason for hiding this comment

turbomam commented Jul 16, 2024 • edited Loading

kheal commented Jul 16, 2024

eecavanna commented Jul 18, 2024 • edited Loading

mslarae13 commented Jul 19, 2024

kheal commented Jul 22, 2024 • edited Loading

Typecode fixes on IDs for `ChromatographyConfiguration`, `StorageProcess`, and `MassSpectrometryConfiguration` class ids #225

Typecode fixes on IDs for `ChromatographyConfiguration`, `StorageProcess`, and `MassSpectrometryConfiguration` class ids #225

kheal commented Jul 16, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024 •

edited

Loading

eecavanna commented Jul 16, 2024 •

edited

Loading

turbomam commented Jul 16, 2024 •

edited

Loading

eecavanna commented Jul 18, 2024 •

edited

Loading

kheal commented Jul 22, 2024 •

edited

Loading