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.

Sign up for GitHub

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

Jump to bottom

Fix range of inSubset to be an IRI, using existing defined subsets. Fixes #1527. #1528

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Fix range of inSubset to be an IRI, using existing defined subsets. Fixes #1527. #1528

merged 3 commits into from
Sep 12, 2024

Conversation

cmungall
Copy link
Member

@cmungall cmungall commented Aug 1, 2024 •
edited
Loading

This fixes #1527:

image

@pbuttigieg
Copy link
Member

What was the issue ? Why does having an IRI improve things ?

@turbomam
Copy link
Contributor

turbomam commented Aug 1, 2024

I haven't looked at the fix, but I am strongly in support of using the most structured representation of subsets... not strings if at all possible.

@cmungall
Copy link
Member Author

cmungall commented Aug 1, 2024

See #1527

  • "things not strings" (https://www.google.com/search?q=thing+not+string)
  • using an IRI is the standard and many tools will ignore the string versions
  • it's consistent with the other subsets that are merged in from imports
  • using an IRI/node means that other metadata such as descriptions can be hung from them
  • the subsets like wwfBiome and envoPolar were already defined in ENVO - but they were not connected to anything. This is pretty confusing for users

matentzn
matentzn approved these changes Aug 2, 2024
View reviewed changes
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.

I reviewed all 1118 changes and they are all as expected!

@turbomam
Copy link
Contributor

turbomam commented Aug 2, 2024

the subsets like wwfBiome and envoPolar were already defined in ENVO - but they were not connected to anything. This is pretty confusing for users

Sold

@cmungall
Copy link
Member Author

cmungall commented Aug 6, 2024

@pbuttigieg ok to merge?

@pbuttigieg pbuttigieg merged commit 3a0dcab into master Sep 12, 2024
2 checks passed
@pbuttigieg pbuttigieg deleted the issue-1527 branch September 12, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turbomam turbomam turbomam approved these changes

@matentzn matentzn matentzn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The range of inSubset should be an IRI, not a literal
4 participants
@cmungall @pbuttigieg @turbomam @matentzn