Improve the definitions #79
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The goal of this PR is to improve the consistency of the simple English definitions * All definitions are lowercase and have no final period * They refer to the concept (the source of the relation) and a "given concept" (the target of the relation) * Similar wording throughout * Fixed some grammatical errors
goodmami
reviewed
Jun 17, 2024
There was a problem hiding this comment.
Thanks! This looks much improved. I have some comments on individual lines and here are a couple of general comments:
- Inconsistent quotation styles for single-line strings; I saw both
"..."and"""...""". This does not change the definitions, though. - You changed several definitions that referred to "words" to instead refer to "concepts". I didn't inspect very closely, but I thought the "word" ones were for sense relations and "concepts" for synset relations, so I hope the change does not lead to confusion.
gwadoc/doc_en.py
Outdated
| relations.simple_aspect_pi.ex.en = "" | ||
|
|
||
|
|
||
| ### Relation Secondary Aspect IP | ||
|
|
||
| relations.secondary_aspect_ip.name.en = "Secondary Aspect (ip)" | ||
| relations.secondary_aspect_ip.df.en = "A concept which is linked to another through a change in aspect (ip)" | ||
| relations.secondary_aspect_ip.df.en = "a concept which is linked to a given concept through a change in aspect (imperfective to perfective)" |
There was a problem hiding this comment.
Is this different from "Simple Aspect (imp to per)"?
There was a problem hiding this comment.
I am not sure what the difference of these relations are. It seems to be something that only really makes sense for Polish and other Slavic languages, so we may have to ask Maciej
gwadoc/doc_en.py
Outdated
| relations.secondary_aspect_ip.ex.en = "" | ||
|
|
||
| ### Relation Secondary Aspect PI | ||
|
|
||
| relations.secondary_aspect_pi.name.en = "Secondary Aspect (pi)" | ||
| relations.secondary_aspect_pi.df.en = "A concept which is linked to another through a change in aspect (pi)" | ||
| relations.secondary_aspect_pi.df.en = "a concept which is linked to a given concept through a change in aspect (perfective to imperfective)" |
There was a problem hiding this comment.
Similarly, is this different from "Simple Aspect (per to imp)"?
Co-authored-by: Michael Wayne Goodman <[email protected]>
Co-authored-by: Michael Wayne Goodman <[email protected]>
Co-authored-by: Michael Wayne Goodman <[email protected]>
Co-authored-by: Michael Wayne Goodman <[email protected]>
|
Thanks for the comments. I cleaned up the quoting strategy I am not sure about the "words" vs "concepts" idea. It was not used consistently before and for example the longer definition of "antonym" talks about "concepts" so I think the change to "concepts" makes everything more consistent. |
|
Hi,
I'm on a train right now with limited access to email, please wait a day or
two. As far as I recall, we were trying to make a difference between
the definitions for synset and sense links (with the difficulty that some
link types, like antonym) are used for both.
I'll comment more when I am back and have had a chance to read things
properly.
…On Tue, 18 Jun 2024 at 18:41, Michael Wayne Goodman < ***@***.***> wrote:
@jmccrae <https://github.com/jmccrae> Thanks, it now looks good to me,
but I'd like to give Francis a chance to comment on the last point.
@fcbond <https://github.com/fcbond>, do you recall if there was anything
intentional behind "words" vs "concepts" in the definitions?
—
Reply to this email directly, view it on GitHub
<#79 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIPZRSEWLWQEDRQNXAOVO3ZIBPLVAVCNFSM6AAAAABJN3ND6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZWGUZTMMRYGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Francis Bond <https://fcbond.github.io/>
|
|
@fcbond Hoping things have calmed down a bit after the summit and conference. Have you had a chance to look at these changes? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this PR is to improve the consistency of the simple English definitions