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.

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

Added strict flag for clean_prefix_map() #353

Merged
merged 2 commits into from
Mar 16, 2023
Merged

Added strict flag for clean_prefix_map() #353

merged 2 commits into from
Mar 16, 2023

Conversation

hrshdhgd
Copy link
Contributor

Fixes #351

As of the current state, if all prefixes in a dataframe are not accounted for in the curie_map, no output is generated and the process errors out.

For now setting the default to be True but I'm not sure if it should be True/False. Thoughts @matentzn ?

@hrshdhgd
Copy link
Contributor Author

@matentzn , this is ready for review based on our discussion here

@hrshdhgd hrshdhgd requested a review from matentzn March 16, 2023 17:18
Copy link
Collaborator

@matentzn matentzn left a comment

Choose a reason for hiding this comment

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

This PR is a no-brainer and can be merged, but it wont solve the OAK problem -the question is who is responsible for filling in missing prefixes?

@hrshdhgd hrshdhgd merged commit c5411d1 into master Mar 16, 2023
@hrshdhgd hrshdhgd deleted the issue-351-2 branch March 16, 2023 17:31
@hrshdhgd
Copy link
Contributor Author

hrshdhgd commented Mar 16, 2023

We could do it here using your solution using the missing IRI but does that mean when the missing IRI is added to the curie_map, all prefixes that aren't accounted for get an additional prefix?

For e.g. let's say ABCD:1234 is unaccounted for. Now curie_map will look like

curie_map:
     BIRNLEX:  http://w3id.org/oak/unknown_prefixes/birnlex

and all mentions of ABCD:XXXX in the dataframe gets converted into BIRNLEX: ABCD:XXXX ?

Assuming it won't be oak in the IRI but something else.

@matentzn
Copy link
Collaborator

I was thinking this:

For e.g. let's say ABCD:1234 is unaccounted for. Currently curie_map looks like

curie_map:
     MONDO:  http://purl.obolibrary.org/obo/MONDO_

Now what you do is this inject the ABC prefix:

curie_map:
     MONDO:  http://purl.obolibrary.org/obo/MONDO_
     ABCD:  http://w3id.org/sssom/unknown_prefix/abcd/

Thats it?

@hrshdhgd
Copy link
Contributor Author

Aaahhh .... much elegant. I likes!

@hrshdhgd
Copy link
Contributor Author

But does that mean the strict flag would be useless at that point?

@matentzn
Copy link
Collaborator

Definetely not. this should only be done in strict=false mode.

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.

clean_prefix_map() should have a strict flag [default = True]
2 participants