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

Resolves #10542 Add fullname of node that is still a PlaceholderNode when assertion fails #10565

Merged

Conversation

TheCleric
Copy link
Contributor

Description

#10542 had a vague error come up due to a PlaceholderNode that did not get replaced. This PR simply adds the full name of the node so that these can be better traced back.

The error was:

...
    assert not isinstance(self.node, PlaceholderNode)
AssertionError

Will now be slightly more helpful:

...
    assert not isinstance(self.node, PlaceholderNode), '%s' % fullname
AssertionError: spacy.ml.models.multi_task.Doc

There's the possibility that something bigger needs to happen here (why is it still a PlaceholderNode?), but this was enough to help me to figure out how to move #10542 forward.

Test Plan

Manually tested with the code provided by @ZeeD from spacy import load ran through mypy with --strict

mypy/nodes.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@hauntsaninja hauntsaninja merged commit 4da09c7 into python:master May 31, 2021
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 this pull request may close these issues.

None yet

2 participants