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

Return special value when words not found? #9

Closed
chengchingwen opened this issue Feb 5, 2018 · 8 comments
Closed

Return special value when words not found? #9

chengchingwen opened this issue Feb 5, 2018 · 8 comments

Comments

@chengchingwen
Copy link
Contributor

Currently WordNet.jl get KeyError when searching for words not in the database,
For example:

julia> db['v', "flew"]
ERROR: KeyError: key "flew" not found
Stacktrace:
 [1] getindex at ./dict.jl:474 [inlined]
 [2] getindex(::WordNet.DB, ::Char, ::String) at /home/peter/.julia/v0.6/WordNet/src/db.jl:20

Will it be better to return something like a empty Lemma for such situation?

@oxinabox
Copy link
Member

oxinabox commented Feb 5, 2018

I don't think so, no.
I think an Error is what should happen.
Can you motivate why the user might not want to be informed by an error?
Any further processing of a fake lemma could lead to incorrect results.
Or later more confusing errors, e.g does the empty lemma be have synset?
does that synset have gloss?

BTW, in this case you wanted to use db['v', "fly"] which is the stem

cross-ref #10

@chengchingwen
Copy link
Contributor Author

Can you motivate why the user might not want to be informed by an error?
...

This is truly a problem, either.

My origin thought was that I need some methods that enable me to handle some out-of-vocabulary words without trying to catch KeyError. I'll say that catching such a general error inside a special function is the last thing I would like to do.

I guess a better way to deal both of problems would be providing some methods that can also set default value?

Or maybe like python's nltk wordnet, leave the synsets function without having to actually touch the Lemma .

like

synsets(db::WordNet.DB, word::String)
synsets(db::WordNet.DB, word::String, pos::Char)

@oxinabox
Copy link
Member

oxinabox commented Feb 5, 2018

I see nothing wrong with:

try
	db['v', "flew"]
catch err
	err isa KeyError || rethrow(err)
	# Handle it however you want
end

KeyError exactly what this is, and its not like that error could be coming from somewhere else inside your try block.

I guess a better way to deal both of problems would be providing some methods that can also set default value?

I agree.
I think db should implement haskey(db, key...), and get(::DB, key, default,
delegating both the the matching functions for db.lemmas (the internal dict)

@chengchingwen
Copy link
Contributor Author

I think db should implement haskey(db, key...), and get(::DB, key, default,
delegating both the the matching functions for db.lemmas (the internal dict)

Does that mean we should implement DB as a subtype of Associative instead of a composite type of Dicts?

@oxinabox
Copy link
Member

oxinabox commented Feb 7, 2018

Does that mean we should implement DB as a subtype of Associative instead of a composite type of Dicts?

Those things are not mutually exclusive.
If it were to be a subtype of Associative, it would still be a composite type of Dicts.
It would just also implement the rest of the Associative interface.

Maybe,
I'm trying to think of the pros and the cons.
One con is that that informal interface is not yet documented,
so it is nontrivial to work out if it is being met correctly.

@chengchingwen
Copy link
Contributor Author

I see.
Maybe at this moment, just implement haskey and get will be a better choice.

@chengchingwen
Copy link
Contributor Author

What about

haskey(db::DB, pos::Char) = haskey(db.lemmas, pos)
haskey(db::DB, pos::Char, word::AbstractString) = haskey(db, pos) ? haskey(db.lemmas[pos], pos) : false

get(db::DB, pos::Char, word::AbstractString, default) = haskey(db, pos, word) ? db.lemmas[pos][word] : default
get(db::DB, word::AbstractString, pos::Char, default) = get(db, pos, word, default)

@oxinabox
Copy link
Member

oxinabox commented Feb 7, 2018

Looks sensible enough to me.
Make a PR?

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

No branches or pull requests

2 participants