wn icon indicating copy to clipboard operation
wn copied to clipboard

Unify Sense relation methods

Open goodmami opened this issue 1 year ago • 2 comments

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 avatar Dec 02 '24 01:12 goodmami