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

PhraseMatcher.add() docs should be more explicit about how to pass a list of Doc objects #4496

Closed
ZeroCool2u opened this issue Oct 21, 2019 · 3 comments
Labels
docs Documentation and website feat / matcher Feature: Token, phrase and dependency matcher

Comments

@ZeroCool2u
Copy link

ZeroCool2u commented Oct 21, 2019

The documentation for PhraseMatcher should be more explicit about how to pass in a list to PhraseMatcher.add().

The documentation specifies the type of the 3rd parameter as *docs or List, which is a bit misleading. *docs is technically correct, but the add() method actually fails when providing a list of Doc objects with the error:

  File "phrasematcher.pyx", line 193, in spacy.matcher.phrasematcher.PhraseMatcher.add
TypeError: an integer is required

Which does not indicate a list cannot be passed. Perhaps it should be more explicit in the example? I lost my morning to this, so I figured I'd make this issue just in case anyone else has the same problem.

Which page or section is this issue related to?

https:/explosion/spaCy/blob/master/website/docs/api/phrasematcher.md

@svlandeg svlandeg added docs Documentation and website feat / matcher Feature: Token, phrase and dependency matcher labels Oct 21, 2019
@ines
Copy link
Member

ines commented Oct 22, 2019

Thanks for the detailed report! And yes, I agree – the type Doc would be more appropriate. Fixing!

(On a related note, I can't wait to finally overhaul the {Phrase}Matcher.add methods for spaCy v3). add(str, List[Doc], on_match=None, **kwargs) is a lot more intuitive.

@ines ines closed this as completed in 4659435 Oct 22, 2019
@ZeroCool2u
Copy link
Author

Thanks Ines, much appreciated! I ended up building spaCy from source trying to figure this out 😐, so maybe I'll actually be able to contribute to v3 now!

@lock
Copy link

lock bot commented Nov 22, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Nov 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
docs Documentation and website feat / matcher Feature: Token, phrase and dependency matcher
Projects
None yet
Development

No branches or pull requests

3 participants