Skip to content

Remove traditionalSynonymyRelationshipType property #206

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

Merged
merged 2 commits into from
Nov 21, 2022

Conversation

nielsklazenga
Copy link
Member

- create GitHub issue (#205)
- create notes, as existing notes were from relationshipType
- make property not required ('required' came from relationshipType as well)
@nielsklazenga
Copy link
Member Author

It would have been better if I had not added the updated markdown/html to the PR. Will not do that anymore in following PRs.

Copy link
Member

@camwebb camwebb left a comment

Choose a reason for hiding this comment

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

Can't approve this quite yet, because I'm not sure we have settled how we will deal with synonymy. I do this this is really important.

master/tcs.yaml Outdated
Relationship Type Vocabulary. In the case of Taxon Concept Relationships
from traditional synonymy, the `relationshipAccordingTo` is the same as the
`accordingTo` of the Taxon Concept that is the `subjectTaxonConcept`.
This term should be used together with `relationshipType` (which is required
Copy link
Member

Choose a reason for hiding this comment

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

... so I can't remember the discussions about this that clearly. The relevant GH discussion seems to be in #65, in particular a potential disagreement (between @Archilegt and you) about whether synonyms should be properties of names or TCs. I agree with you that to make a synonymy statement one must have a TC to work with, and in general the most accurate way to encode synonymy is using TC intersects TC. But... in practice most synonymy statements have no "sec." for the names so they appear to be properties of names. Have we discussed writing any general guidance for users which would allow/encourage TC name X; accordingTo NULL? I.e., forcing a name to be a concept, but with unknown TaxonTreatment. If we will allow this, then it makes sense to model synonym as a TC property. If this is not allowed, we must make synonymy a property of names, otherwise vast amounts of taxonomy information cannot be correctly encoded.

@nielsklazenga
Copy link
Member Author

nielsklazenga commented Nov 11, 2022

@camwebb , this is not about whether we have the property or not – I'd rather not have it either – but about fixing errors in the text. The property is there with or without this PR, but at the moment it is required and has the notes of relationshipType. If only I could add and remove terms from the term list simply by doing a PR. I have copied your comment to issue #205, as I want to have the discussion too (but not here). I cannot remember why I did not just merge the PR but asked for a review. Probably because it was the first one I did and I felt that since I had asked for a PR I could not just go ahead and merge it.

- term is nor ready to be included yet.
@nielsklazenga
Copy link
Member Author

@camwebb , I have now added a commit that does what I wanted to do in the first place.

@nielsklazenga nielsklazenga changed the title change traditionalSynonymyRelationshipType property Remove traditionalSynonymyRelationshipType property Nov 21, 2022
@nielsklazenga nielsklazenga merged commit 652b135 into master Nov 21, 2022
@nielsklazenga nielsklazenga deleted the property-traditionalSynonymyRelationshipType branch November 21, 2022 04:32
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.

2 participants