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

Go site 676 gorule 0000022 check for retracted publications #674

Merged

Conversation

mugitty
Copy link
Collaborator

@mugitty mugitty commented May 9, 2024

No description provided.

@mugitty mugitty requested review from dustine32 and kltm May 9, 2024 20:47
kltm
kltm previously requested changes May 9, 2024
Copy link
Member

@kltm kltm left a comment

Choose a reason for hiding this comment

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

@mugitty These changes generally make sense to me.
Looking at this, one thing we had discussed was making the use of retracted publications completely optional--i.e. adding a new option to the path and only executing if available. It looks like this just depends on the go-site metadata directory? We would like to be able to turn these on/off, switch targets, etc.; it looks like it errors out on file not existing now?

@kltm
Copy link
Member

kltm commented May 9, 2024

@mugitty It looks like the test error:

==================================== ERRORS ====================================
_______________ ERROR collecting tests/test_validation_rules.py ________________
tests/test_validation_rules.py:1: in <module>
    from ontobio.validation import rules
ontobio/validation/rules.py:11: in <module>
    from ontobio.validation import metadata
ontobio/validation/metadata.py:188: in <module>
    def retracted_pub_set(metadata)->set[str]:
E   TypeError: 'type' object is not subscriptable
=============================== warnings summary ===============================

is related to the handling of go-site metadata as mentioned above.

@mugitty
Copy link
Collaborator Author

mugitty commented May 9, 2024

@kltm , I did not realized that we wanted a flag to determine if the test should be run. So, it would only be in ontobio-parse-assocs.py as a parameter?

For the error, is it expecting set[] instead of set[str] due to version of python?

@kltm
Copy link
Member

kltm commented May 9, 2024

@mugitty For me, I just want to make sure that the file is optional, and ideally be able to specify it in different contexts. The exact mechanism isn't too too important.

@dustine32
Copy link
Collaborator

Linking to issue geneontology/go-site#676

@mugitty mugitty requested review from kltm and removed request for kltm May 10, 2024 23:49
@mugitty mugitty dismissed kltm’s stale review May 13, 2024 17:39

Updated as requested

Copy link
Member

@kltm kltm left a comment

Choose a reason for hiding this comment

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

Examining code, it looks like:

  • if file given, rule run
  • if file found at assumed location, rule run
  • otherwise, rule skipped

I believe this is in line with what we discussed; if so, please merge at your convenience

@mugitty mugitty merged commit 682e2f5 into master May 16, 2024
5 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