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

Unify Sense relation methods #225

Open
goodmami opened this issue Dec 2, 2024 · 2 comments
Open

Unify Sense relation methods #225

goodmami opened this issue Dec 2, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@goodmami
Copy link
Owner

goodmami commented Dec 2, 2024

Is your feature request related to a problem? Please describe.

In WN-LMF, the targets of <SenseRelation> child elements of <Sense> elements can be either a Sense or a Synset. Wn imports these into separate tables and makes them available via separate methods: Sense.get_related() and Sense.get_related_synsets() (currently, Sense.relations() only works for relations with sense targets). It might make sense to merge these into Sense.get_related() with a parameter to choose the target type:

    def get_related(self, *args, target_type: str = "sense") -> list[Union['Sense', Synset]]:
        ...

The value of target_type is a string (or a StrEnum) with values "sense", "synset", and "both". Initially, target_type will default to "sense" and Sense.get_related_synsets() will call Sense.get_related(..., target_type="synset"), but later we could change the default to "both" and deprecate Sense.get_related_synsets().

Additionally, other relation methods like Sense.relations() can use this parameter to fill a feature gap.

Describe the solution you'd like

See above.

Describe alternatives you've considered

We could continue to have two functions for each (.get_related_synsets(), .synset_relations(), etc.). This seems unnecessary.

Additional context

Along with planning for the new .relation_map(), this would prevent two new functions when one could suffice.

@goodmami goodmami added the enhancement New feature or request label Dec 2, 2024
@goodmami
Copy link
Owner Author

goodmami commented Dec 4, 2024

@fcbond Can you remind me why we have such a thing as Sense–Synset relations in WN-LMF? I looked through all senses in OMW, all versions of EWN/OEWN, OWN-PT, and Odenet, and couldn't find a single instance:

>>> len(wn.senses())
3407588
>>> sum(len(s.get_related_synsets()) for s in wn.senses())
0

I'm guessing there is some wordnet (maybe the Polish one?) that links senses to synsets with domain_topic, domain_region, exemplifies, or other relations, but maybe the feature has always just been aspirational?

If we don't foresee an immediate need for Sense–Synset relations, I'd suggest we remove support for them from Wn and maybe from WN-LMF. Trying to accommodate them makes the code much more complicated than it needs to be.

@goodmami
Copy link
Owner Author

It looks like there's at least interest in better supporting them in OEWN: globalwordnet/english-wordnet#957

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant