Skip to content

Add prefix: celldive, separating it out from dsmz - #1478

Open
nagutm wants to merge 12 commits into
biopragmatics:mainfrom
nagutm:celldive
Open

Add prefix: celldive, separating it out from dsmz#1478
nagutm wants to merge 12 commits into
biopragmatics:mainfrom
nagutm:celldive

Conversation

@nagutm

@nagutm nagutm commented Mar 22, 2025

Copy link
Copy Markdown
Collaborator

Based on the discussion in #1454, I renamed the old dsmz prefix which was too vague to celldive which better reflects the specific semantic space that the metadata was describing.

I updated the description, mappings, regex pattern and examples to reflect this change.

@nagutm nagutm mentioned this pull request Mar 22, 2025
Comment thread src/bioregistry/data/bioregistry.json
@codecov

codecov Bot commented Mar 22, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.16%. Comparing base (8950e70) to head (51e622d).
⚠️ Report is 1185 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1478      +/-   ##
==========================================
+ Coverage   42.51%   47.16%   +4.64%     
==========================================
  Files         117      122       +5     
  Lines        8327     8876     +549     
  Branches     1963     1495     -468     
==========================================
+ Hits         3540     4186     +646     
+ Misses       4582     4434     -148     
- Partials      205      256      +51     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

}
},
"dsmz": {
"cellosaurus": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it looks like a lot of components got lost, please make sure they are restored. How did you do the rename exactly? When I have done this, I just change the prefix, add the old prefix to the synonym list, then run bioregistry lint

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I made the following changes when I renamed it:

  • removed the re3data mapping which was not mapping to CellDive, but DSMZ in general
  • removed example_extras because only the identifiers with ACC- format were for cell lines. For example, DSM-2 is for Paenibacillus macquariensis subsp. macquariensis, PV-0998 is for Apple chlorotic leaf spot virus ACLSV, etc.
  • Updated the regex pattern to only include ACC which is what covers cell lines.

The other identifier formats that are not ACC are related to specific microorganisms which felt like it was out of the scope for the celldive prefix so I intentionally removed them. Should these be maintained under a separate prefix or is it better to maintain this as part of the celldive prefix?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ahh I see what is happening. so we should probably maintain a superset dsmz prefix that is a union of all of these different vocabularies, since in practice they are conflated all together (and the ACC-, PV-, etc. work as janky subspaces)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In that case I can modify this PR to just be about adding the celldive prefix and create a new PR that modifies the existing dsmz prefix accordingly.

@nagutm nagutm changed the title Change dsmz to celldive and update metadata Add prefix: celldive Mar 26, 2025
@bgyori

bgyori commented Mar 29, 2025

Copy link
Copy Markdown
Contributor

In that case I can modify this PR to just be about adding the celldive prefix and create a new PR that modifies the existing dsmz prefix accordingly.

I actually think the right scope for this PR would include any changes to dsmz that are directly related to the addition of the new celldive prefix. Otherwise, we would end up with two mappings to cellosaurus:DSMZCellDive, one from the new celldive and one from the existing dsmz which doesn't make sense.

@bgyori bgyori changed the title Add prefix: celldive Add prefix: celldive, separating it out from dsmz Mar 29, 2025
@bgyori bgyori added Prefix New Used in combination with prefix, metaprefix, or collection for new entries labels Mar 29, 2025
@bgyori

bgyori commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

I updated the description for celldive to mention DSMZ explicitly, and added reviewer info. However, I think this needs a bit more investigation. Basically, there are multiple identifiers involved in CellDive, and the URL pattern on the dedicated CellDive website is not the same as the identifier that is resolved via the core DSMZ website. The PR as it currently stands is related to the ACC- identifiers that are DSMZ-related rather than what is called the CellDive ID listed in the table in the attached screenshot or the name appearing in the URL pattern.

image

@bgyori

bgyori commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

It is important to this decision that it turns out, there are many cell lines in CellDive that do not have ACC- identifiers in DSMZ.

Here is a snippet of such entries
image

with one specific example which has CellDive ID 910 but no DSMZ ID.
image

Based on this, I would be tempted to change the PR to cover what is actually called the CellDive ID, with the caveat that the IDs cannot be resolved via a URL pattern since the URL patterns use the cell line name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Used in combination with prefix, metaprefix, or collection for new entries Prefix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants