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

Bugfix: clean_prefix_map() not detecting prefix synonyms #485

Closed
wants to merge 6 commits into from
Closed

Bugfix: clean_prefix_map() not detecting prefix synonyms #485

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
764dc1e
Extended prefix maps usage updates
joeflack4 Jan 13, 2024
57dd171
Switch to curies impl
cthoyt Jan 16, 2024
2a59f73
Update poetry.lock
cthoyt Jan 16, 2024
565dd0f
Clean up with new curies functionality
cthoyt Jan 17, 2024
6790d7d
Reverted to essential EPM bugfix
joeflack4 Jan 20, 2024
8b7ff6b
Adding test to illustrate problem
matentzn Jan 20, 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
23 changes: 23 additions & 0 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,29 @@ def test_get_dict_from_mapping(self):
self.assertEqual(value, result_with_mapping_object[key])
self.assertEqual(value, result_with_dict[key])

def test_converter_get_prefixes_including_synonyms_and_get_subconverter(self):
"""Test if get_subconverter() works with prefix synonyms."""
EPM = [
{
"prefix": "SCTID",
"prefix_synonyms": ["snomed"],
"uri_prefix": "http://snomed.info/id/",
},
]

converter = Converter.from_extended_prefix_map(EPM)
self.assertNotIn("snomed", converter.get_prefixes(include_synonyms=False))
self.assertIn("snomed", converter.get_prefixes(include_synonyms=True))
Copy link
Member

Choose a reason for hiding this comment

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

it's not SSSOM-py's job to test curies functionality. The test should show how the change in this PR actually effects input and output to SSSOM-py functions

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not a test, it's some code to show why the pr the way it is should not be merged.. I don't intend to add this as a test here

subconverter = converter.get_subconverter(["snomed"])
self.assertIn(
"snomed",
subconverter.bimap.keys(),
"The subconverter which is used to generate "
"the prefixmap for the serialisation does not "
"contain the 'snomed' prefix, but it is used in "
"the raw data",
)

def test_curiechain_with_conflicts(self):
"""Test curie map with CURIE/URI clashes."""
PREFIXMAP_BOTH = {
Expand Down
Loading