The Wayback Machine - https://web.archive.org/web/20200911042551/https://github.com/nltk/nltk/issues/2273
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

Max depth of the all wordnet POS should be returned and kept as static #2273

Open
alvations opened this issue Apr 22, 2019 · 3 comments
Open

Max depth of the all wordnet POS should be returned and kept as static #2273

alvations opened this issue Apr 22, 2019 · 3 comments

Comments

@alvations
Copy link
Contributor

@alvations alvations commented Apr 22, 2019

The _compute_max_depth() used for lch_similarity() returns None regardless of POS :

from nltk.corpus import wordnet as nltk_wn
from nltk.corpus.reader.wordnet import POS_LIST

for pos in POS_LIST:
    print(pos, 
          nltk_wn._compute_max_depth(pos, simulate_root=True), 
          nltk_wn._compute_max_depth(pos, simulate_root=False))
          print(pos, nltk_wn._max_depth)

[out]:

n None None
n defaultdict(<class 'dict'>, {'v': 12, 'a': 0, 'r': 0, 'n': 19})
v None None
v defaultdict(<class 'dict'>, {'v': 12, 'a': 0, 'r': 0, 'n': 19})
a None None
a defaultdict(<class 'dict'>, {'v': 12, 'a': 0, 'r': 0, 'n': 19})
r None None
r defaultdict(<class 'dict'>, {'v': 12, 'a': 0, 'r': 0, 'n': 19})

The self._compute_max_depth doesn't return anything, my suggestion is to expose it as a public function and let it return the depth while setting the max depth.


Another point is that the max_depth will call the all_synsets() once and this is costly, it could be saved as static value from the start and access to the integer would be much simpler.

@alvations alvations added the wordnet label Apr 22, 2019
@alvations alvations changed the title Max depth of the all wordnet POS are None Max depth of the all wordnet POS should be returned Apr 22, 2019
@alvations alvations changed the title Max depth of the all wordnet POS should be returned Max depth of the all wordnet POS should be returned and kept as static Apr 22, 2019
@EeshitaBiswas
Copy link
Contributor

@EeshitaBiswas EeshitaBiswas commented Sep 30, 2019

Can I start working on this?

@EeshitaBiswas
Copy link
Contributor

@EeshitaBiswas EeshitaBiswas commented Oct 11, 2019

Any update on this?

@alvations
Copy link
Contributor Author

@alvations alvations commented Oct 11, 2019

Actually this is already fixed in https://github.com/nltk/wordnet , we're still looking at how to integrate the stand-alone library without breaking existing usage in the main nltk library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.