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

Addressing issue 1304 of malformed definition source links #1309

Merged
merged 2 commits into from Apr 7, 2022

Conversation

ddooley
Copy link
Contributor

@ddooley ddooley commented Apr 1, 2022

Removed definition source links which were all of form
42169:AnnotationAssertion(Annotation(http://www.geneontology.org/formats/oboInOwl#hasDbXref http://purl.obolibrary.org/obo/ENVO_) ...

@kaiiam
Copy link
Contributor

kaiiam commented Apr 4, 2022

This looks good to me thanks @ddooley for manually cleaning this up! @cmungall @wdduncan @pbuttigieg I think this is good to merge.

src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
src/envo/envo-edit.owl Show resolved Hide resolved
@wdduncan
Copy link
Member

wdduncan commented Apr 6, 2022

@ddooley I also noticed that the classes you added use the created_by annotation. The wiki says to use dc:creator. Although, I think this should say dcterms:creator.

What is your opinion @kaiiam @pbuttigieg

@cmungall cmungall self-requested a review April 6, 2022 02:00
Copy link
Member

@cmungall cmungall left a comment

Choose a reason for hiding this comment

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

Let's merge ASAP. This fixes a semi-urgent technical issue with a previous PR

Let's carry forward discussion of the precise wording of definitions into a new issue/PR

@kaiiam
Copy link
Contributor

kaiiam commented Apr 6, 2022

Thanks @wdduncan everything you identified could be cleaned up as you describe but as @cmungall said it's not an urgent technical fix. I'd say if someone has scope to clean this quickly before merging great if not as @cmungall says we should perhaps just merge.

@wdduncan
Copy link
Member

wdduncan commented Apr 6, 2022

@cmungall @kaiiam I understand what you are saying, but I worry that if the issues aren't addressed now, they never will be. The fixes don't seem that difficult.

@ddooley How long do you think it would take to address them?

@kaiiam
Copy link
Contributor

kaiiam commented Apr 6, 2022

Fair point @wdduncan if @ddooley you can quickly address Bill's comments above that would be ideal!

@ddooley
Copy link
Contributor Author

ddooley commented Apr 6, 2022

i can do the last changes this afternoon directly on the OWL file. I didn't realize yesterday that you'd provided a whole batch of feedback, Duncan, much appreciated!

@wdduncan
Copy link
Member

wdduncan commented Apr 6, 2022

Thanks for getting to this so quickly @ddooley !!!

We'll merge the merge once your finished :)

@ddooley
Copy link
Contributor Author

ddooley commented Apr 7, 2022

About the "created_by" annotation, I'm seeing quite a number of entries have this besides the ones I recently added. If you want to normalize created_by to dc/terms:creator I'm fine with that but lets do that as a separate pr for bulk search and replace?!
That said, I think its ready to merge.

and Ophthalmologist misspelling!
Copy link
Contributor

@kaiiam kaiiam left a comment

Choose a reason for hiding this comment

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

Looks good @ddooley. I agree we should deal with the created_by APs in bulk as we have multiple APs in use at the moment.

@wdduncan wdduncan merged commit 903e1ee into master Apr 7, 2022
@wdduncan
Copy link
Member

wdduncan commented Apr 7, 2022

Thanks for edit @ddooley !
PR merged :)

@kaiiam
Copy link
Contributor

kaiiam commented Apr 7, 2022

Thanks @wdduncan!

@pbuttigieg pbuttigieg deleted the envo-issue-1304 branch August 15, 2022 16:23
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.

None yet

4 participants