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

tree should check word exists #13

Closed
jmsv opened this issue May 25, 2018 · 2 comments · Fixed by #34
Closed

tree should check word exists #13

jmsv opened this issue May 25, 2018 · 2 comments · Fixed by #34

Comments

@jmsv
Copy link
Owner

jmsv commented May 25, 2018

$ ety asdfghjkl -t
asdfghjkl (English)

Always outputs '(English)', since it's the default language. The word should be looked up in the data and not displayed if missing

@alxwrd
Copy link
Collaborator

alxwrd commented Jun 12, 2018

Just for reference: the command line appears to be fixed, and correctly displays:

$ python -m ety -t asfgds
No origins found for word: 'asfgds'

but the api still generates a Tree with one node.

>>> print(ety.tree("asfgds"))
asfgds (English)

Not sure what the correct return value should be if the word has no origins. Maybe None? Logically as .origins() returns an empty to list, I would say .tree() should return an empty tree. But, it's not possible to return an empty Tree as trying to print it throws an error:

>>> print(Tree())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/treelib/tree.py", line 120, in __str__
    self.__print_backend(func=write)
  File "/treelib/tree.py", line 170, in __print_backend
    line_type):
  File "/treelib/tree.py", line 199, in __get_iter
    raise NodeIDAbsentError("Node '%s' is not in the tree" % nid)
treelib.exceptions.NodeIDAbsentError: Node 'None' is not in the tree

Is it also a possibility with the planned changes from #24 that when a Word object is created, we could check to see if it's valid. But then what about leaf nodes.

@jmsv
Copy link
Owner Author

jmsv commented Jun 12, 2018

Hmm, None makes sense imo. I considered raising a ValueError but that seems like a worse idea

If we went with this behaviour, it might also make sense to also make origins return None, rather than an empty list

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 a pull request may close this issue.

2 participants