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

Fix GoldParse init #4524

Merged
merged 1 commit into from
Oct 25, 2019
Merged

Conversation

svlandeg
Copy link
Member

@svlandeg svlandeg commented Oct 25, 2019

The GoldParse init constructor has a catch-all **_ which seems to have led to a subtle bug in nn_parser.pyx.

Personally I would prefer removing **_ from the constructor, but this may break people's scripts (though the breaking might be informational). We can also remove it as part of the v.3.0 release. Unless there is a reason to keep this?

Types of change

bug fix

Checklist

  • I have submitted the spaCy Contributor Agreement.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@svlandeg svlandeg added bug Bugs and behaviour differing from documentation feat / parser Feature: Dependency Parser labels Oct 25, 2019
@honnibal
Copy link
Member

I think we had the catch-all there for some backwards compatibility at some point. Removing it seems fine.

@honnibal honnibal merged commit 99e309b into explosion:master Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs and behaviour differing from documentation feat / parser Feature: Dependency Parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants