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

ValidationError is not Hashable (2.0.0) #452

Closed
Guibod opened this issue Sep 29, 2016 · 3 comments
Closed

ValidationError is not Hashable (2.0.0) #452

Guibod opened this issue Sep 29, 2016 · 3 comments

Comments

@Guibod
Copy link

Guibod commented Sep 29, 2016

I can't logging.exception(ValidationError) it seems that the object is not hashable.

That's sad, it used to work pretty well on 1.1. I'm using Python 3.4.

class TestUnhashable(unittest.TestCase):
    def test_type_error_is_hashable(self):
        try:
            raise TypeError('Foo bar')
        except TypeError as e:
            logging.exception(e)

    def test_validation_error_is_unhashable(self):
        try:
            raise ValidationError('Foo bar')
        except ValidationError as e:
            with self.assertRaises(TypeError) as c:
                logging.exception(e)

            self.assertIsInstance(c.exception, TypeError)
            self.assertEqual(c.exception.args[0], 'unhashable type: \'ValidationError\'')
@Guibod
Copy link
Author

Guibod commented Sep 29, 2016

Correction. I have no clue if it was working in v1.1. Yet this would be great to be able to logging.exception(ValidationError)

@lkraider
Copy link
Contributor

lkraider commented Mar 16, 2017

Your code works in Python 2, but breaks in Python 3 because there the language auto-sets __hash__ = None when __eq__ is defined for a class (as is the case in Schematics ErrorMessage). You are forced to provide your own hash method in that case.

In Python an object can only be hashable if it has a hash value which never changes during its lifetime, that is, it is immutable.

We have a "problem" right now with the ErrorMessages in that they are mutable (they contain a list of messages and a pop method that changes it).

Also, hashable objects which compare equal must have the same hash value, and we use the instance __dict__ for comparison inside __eq__, which is also mutable/unhashable.

Python itself does not require Exceptions to be immutable. At least in 2002, GVR was in favor of mutable exceptions: https://mail.python.org/pipermail/python-dev/2002-August/027900.html

The logging framework assumes exceptions are hashable (and thus immutable), which is an undocumented assumption. But since logging is in the stdlib, we need to play by their rules.

The fix to support logging is to make all schematics exceptions immutable. I don't have a proposal for how to do this, but would welcome a PR.

toumorokoshi added a commit to toumorokoshi/schematics that referenced this issue Mar 18, 2017
issue schematics#452: all exception types are now hashable and immutable
issue schematics#445: a human readable and primitive only option is available (e.g. for json serialization)
issue schematics#369: follows PEP-352 conventions, and normalizes all errors found in an .errors attribute
lkraider pushed a commit that referenced this issue Mar 29, 2017
* Enhancing and addressing some issues around exceptions:

issue #452: all exception types are now hashable and immutable
issue #445: a human readable and primitive only option is available (e.g. for json serialization)
issue #369: follows PEP-352 conventions, and normalizes all errors found in an .errors attribute

* string representations of error messages are equivalent to json formatting a primitive representation. This representation was chosen for legibility and simplicity. It's an implementation detail to serialize to json, and shouldn't be relied on. (serializing `to_primitive` exists for that purpose)

* errors dicts and lists are converted to FrozenDict and FrozenList. Although they might contain mutable data inside, we expect that indicating the base container as immutable users will understand not to mutate data inside (it's not clear what problems it would cause, the goal is simply to provide an immutable interface for the Schematics errors).

* exceptions instantiation now with a single parameter, that is the error object (string, list, dict or other).

* exceptions `repr` can be `eval`d.
@lkraider
Copy link
Contributor

Fixed in #477

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