Skip to content
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.

Sign up for GitHub

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

fix!: ConceptMapping.coding.code should be a CURIE #390

Merged
merged 2 commits into from
Dec 31, 2024
Merged

fix!: ConceptMapping.coding.code should be a CURIE #390

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed970c2
fix!: `ConceptMapping.coding.code` should be a CURIE
korikuzma Dec 31, 2024
89508f4
update schemas
korikuzma Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/gene/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ def _create_concept_mapping(
``NamespacePrefix``
:return: Concept mapping for identifier
"""
source, source_id = concept_id.split(":")
source = concept_id.split(":")[0]

try:
source = NamespacePrefix(source)
Expand All @@ -424,10 +424,9 @@ def _create_concept_mapping(
raise ValueError(err_msg) from e

system = NAMESPACE_TO_SYSTEM_URI.get(source, source)
code_ = concept_id.upper() if source == NamespacePrefix.HGNC else source_id

return ConceptMapping(
coding=Coding(code=code(code_), system=system), relation=relation
coding=Coding(code=code(concept_id), system=system), relation=relation
Comment on lines -427 to +429
Copy link
Member Author

Choose a reason for hiding this comment

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

@jsstevenson is it fine to just use the concept_id as the CURIE? Technically, HGNC uses uppercase. I might be overthinking things with mappings

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, maybe we just need to look at updating our NamespacePrefix member values in the future?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think that'd be simplest.

Realistically there is cannot be a "true" CURIE prefix for a given source (identifiers.org was supposed to be the solution, but then it has its own problems so people made the bioregistry, but it has problems and so on and so on), which is why Matt was talking about some kind of local prefix expansions document. Some sources obviously have their own personal opinions about what prefixes you use for them, but then it begs the question of what to do if there's a community standard which differs from that. We should definitely look at updating our own prefix mapping where it makes sense though.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jsstevenson Ah okay, maybe we wait for Matt (and others) to provide guidance then and update our NamespacePrefix values whenever that discussion happens?

)

gene_obj = MappableConcept(
Expand Down
Loading
Loading