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 ErrorsWithCodes().__class__ return value #5436

Merged
merged 3 commits into from
May 18, 2020

Conversation

ilivans
Copy link
Contributor

@ilivans ilivans commented May 14, 2020

Description

Fix ErrorsWithCodes().__class__ value - it should be a class rather than a string like "[__class__] .." before.
It's critical for functions which try to inference the class of an object, i.e. isinstance
I personally encountered the issue using rollbar which collects all the visible variables and tries to serialize those. In case of ErrorsWithCodes() it failed during isinstance(o, collections.abc.Mapping) check.

I ran all the tests using Python 3.8, Ubuntu 19.04, no spacy models preinstalled.
spacy is installed with pip install -e path_to_repo.

FAILED spaCy/spacy/tests/lang/fa/test_noun_chunks.py::test_noun_chunks_is_parsed_fa - Failed: DID NOT RAISE <class 'ValueError'>
======================== 1 failed, 1709 passed, 890 skipped, 49 xfailed, 1 xpassed, 4 warnings in 69.88s (0:01:09) =========================

*The failed test case fails without my change too

Types of change

A 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.

@ilivans ilivans changed the title Fix ErrorsWithCodes().__class__ return value Fix ErrorsWithCodes().__class__ return value May 14, 2020
@adrianeboyd
Copy link
Contributor

Huh, I don't know why the CI didn't fail before on that test, very weird, but thanks for the quick catch!

@ines ines added the bug Bugs and behaviour differing from documentation label May 18, 2020
@ines ines merged commit a41e28c into explosion:master May 18, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants