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

Dec 2023 infores updates for Service Provider/BTE #1429

Merged
merged 19 commits into from
Dec 15, 2023

Conversation

colleenXu
Copy link
Contributor

Still working on this.

Need to discuss with @sierra-moxon when this PR needs to be ready, so these changes can be used in this release cycle.

@colleenXu
Copy link
Contributor Author

Note

I've been creating and editing wiki pages for the infores entries I'm adding/updating here (see comments in the linked issues).

However, I noticed that tursynay appeared to have edited many pages into being a common format.

So I've reviewed and edited all of the wiki pages I was working on, trying to match the format tursynay was using. This means removing the licensing info, but this info is still saved in the page history.

@sierra-moxon
Copy link
Member

@colleenXu - if you merge the master branch into this fork, you should see a new makefile target called validate_infores (e.g. from the command line, make validate_infores) that will run through the infores_catalog.yaml file looking for errors. It'll run automatically on PR updates so you can just view any failures here in github as well.

using the condensed set of knowledge types
I didn't make guesses for resources that I felt other consortium members would know more about (go, monarchinitiative, rampdb, ncats rare-source, etc)
@colleenXu colleenXu force-pushed the CX-dec2023-infores-updates branch from 15681f9 to 186f5f4 Compare December 14, 2023 08:27
@colleenXu
Copy link
Contributor Author

I've added two commits with my best guesses for knowledge-level (out of the condensed set of knowledge-types, see Translator Slack convo) :

  • one for new entries
  • one for existing entries. But I didn't cover resources that I felt other consortium members would know more about (go, monarchinitiative, rampdb, ncats rare-source, etc)

@sierra-moxon I tried syncing my fork (this branch and its master) with the current master. But it's not clear to me if the validate_infores ran...

@sierra-moxon
Copy link
Member

sierra-moxon commented Dec 14, 2023

@colleenXu - let's set it to "Ready for review" (its currently in "Draft").
https://github.com/biolink/biolink-model/actions/runs/7206368766/job/19631050697?pr=1429 shows the output of make validate_infores - and these changes were successful. :) I had just a few comments on the knowledge_level changes in the PR. If we can address them today, we can include this in the last release of Biolink in this Translator release. From TACT this morning: going forward, we will be moving the infores catalog to its own repo in this org to decouple it from biolink releases (but we will need to make TACT aware of new sources, data ingests, KP additions, etc. as represented in this PR during the release planning stage).

infores_catalog.yaml Outdated Show resolved Hide resolved
infores_catalog.yaml Outdated Show resolved Hide resolved
infores_catalog.yaml Outdated Show resolved Hide resolved
@sierra-moxon sierra-moxon marked this pull request as ready for review December 14, 2023 17:38
@colleenXu
Copy link
Contributor Author

@sierra-moxon I don't have strong opinions on the knowledge-level guesses I made. If you'd like me to change them, I can...

colleenXu referenced this pull request in colleenXu/biolink-model Dec 15, 2023
so biothings apis, some embl-ebi apis brought in by service provider

based on Matt's view that knowledge-level should reflect the content/underlying source https://github.com/biolink/biolink-model/pull/1429\#discussion_r1427313856
@colleenXu colleenXu force-pushed the CX-dec2023-infores-updates branch from 2278dc7 to 2c84c14 Compare December 15, 2023 01:03
@sierra-moxon
Copy link
Member

Thanks @colleenXu - this looks great to me - shall we get it in before code freeze tomorrow?

@colleenXu
Copy link
Contributor Author

colleenXu commented Dec 15, 2023

@sierra-moxon @mbrush Following what Matt Brush said above, I've added a commit so the intermediate apis I reviewed (biothings, embl-ebi apis Service Provider brings in) have knowledge-level terms that reflect the content/underlying sources.

I'm marking as-resolved the two biothings semmeddb threads...

@colleenXu
Copy link
Contributor Author

@sierra-moxon I'd like it to get in before code freeze!

I notice the "chebi" conversation isn't resolved yet....did you want me to make a change there?

@sierra-moxon
Copy link
Member

I see why you did that for ChEBI, thank you and I agree with you. I think at some level, ALL the sources are mixed. When we're able to assign knowledge_level on a per-edge basis, it would make sense if that edge from an automated pipeline import into ChEBI had a different knowledge level than subclass_of edges between ontology terms -- in fact, I might want to add a CV term to the list of knowledge levels, something like "inferred from automated pipeline". (or something better/shorter than that).

Also, I think this is exactly the kind of feedback we're looking for on this effort -- how easy/hard it is to assign these parameters to full sources vs individual edges. We should add it to the EPC agendas for use next round. For now, I would vote to revert (if for no other reason than to keep ontologies in the infores catalog consistent).

@colleenXu
Copy link
Contributor Author

@sierra-moxon okay, it's changed back now. Is anything else needed?

@sierra-moxon
Copy link
Member

perfect! lets merge!

@sierra-moxon sierra-moxon merged commit 06a0de2 into biolink:master Dec 15, 2023
6 checks passed
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.

3 participants