mapping-commons / sssom-py

Python toolkit for SSSOM mapping format
https://mapping-commons.github.io/sssom-py/index.html#
MIT License
48 stars 10 forks source link

Migrate sssom validation to new LinkML validation framework #532

Closed matentzn closed 4 days ago

matentzn commented 2 months ago

Blocked by:

Note to future self, only thing that stands in the way of this PR now is #2164, if that is dealt with

matentzn commented 2 months ago

@pkalita-lbl Thanks a ton for your help, I made issues for everything and will resume work here once they are resolved. Thank you!

matentzn commented 1 week ago

@hrshdhgd I assigned this to you for review. I think it is done, but I am a bit scared of how the changes will impact people relying on validation downstream, so I prefer to have your eyes on it. Thank you! Ideally this should be a careful review beyond "lgtm" as I feel I cant afford messing this up!

matentzn commented 5 days ago

@hrshdhgd just to be sure I need a positive approval from you for this PR, and real certainty that all is understood and correct. Can you comment on the exact pieces of code you have questions about?

hrshdhgd commented 4 days ago

real certainty that all is understood and correct.

That is exactly what I'm trying to do as per my previous comment.

matentzn commented 4 days ago

So the idea here is if validation encounters a SSSOM tsv file does not contain (say) a valid column value, the row is dropped and warning is logged. Correct?

I hope this PR is only affecting the sssom validate command, and no other functionality. Did you see any possibility of this PR affecting any other methods, like sssom parse? That would make me even more cautious :D