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

Generate a hash function when allow_mutation is False #1880

Closed
rhuille opened this issue Aug 30, 2020 · 4 comments · Fixed by #1881
Closed

Generate a hash function when allow_mutation is False #1880

rhuille opened this issue Aug 30, 2020 · 4 comments · Fixed by #1881

Comments

@rhuille
Copy link
Contributor

rhuille commented Aug 30, 2020

Feature Request

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.6.1
            pydantic compiled: False
                 install path: /home/raphael/rhuille/pydantic/pydantic
               python version: 3.8.5 (default, Aug 18 2020, 10:43:36)  [GCC 7.5.0]
                     platform: Linux-5.0.0-1067-oem-osp1-x86_64-with-glibc2.27
     optional deps. installed: ['typing-extensions', 'email-validator', 'devtools']

Hi there,

I need the model to be hashable when I set allow_mutation to False. I could write myself a __hash__ function, but I think it could be nice if pydantic generated it by default in the class BaseModel.

This would be the same behavior of the built-in dataclass which generate a hash function if the object is immutable (parameter frozen is True) and the __eq__ function exists.

Quoting the documentation https://docs.python.org/3/library/dataclasses.html :

If eq and frozen are both true, by default dataclass() will generate a __hash__() method for you.

We could generate a default __hash__ function for BaseModel if allow_mutation is False, what do you think ?

Examples

For example, now we have:

from pydantic import BaseModel

class A(BaseModel):
    x: int
    
    class Config:
        allow_mutation = False

a = A(x=1)
d = {a: 2}

>>> TypeError: unhashable type: 'A'

After, we would have:

from pydantic import BaseModel

class A(BaseModel):
    x: int
    
    class Config:
        allow_mutation = False

a = A(x=1)
d = {a: 2}
d[a]
>>> 2

(Note that if allow_mutation = True, the hash function would be None and the behavior would not be changed)

rhuille added a commit to rhuille/pydantic that referenced this issue Aug 30, 2020
The generated hash is the hash of the dict_value
of the BaseModel.__dict__ attribute.

closes pydantic#1880
rhuille added a commit to rhuille/pydantic that referenced this issue Aug 30, 2020
The generated hash is the hash of the dict_value
of the BaseModel.__dict__ attribute.

closes pydantic#1880
rhuille added a commit to rhuille/pydantic that referenced this issue Aug 30, 2020
The generated hash is the hash of the dict_value
of the BaseModel.__dict__ attribute.

closes pydantic#1880
@jolynch
Copy link

jolynch commented Mar 1, 2021

@rhuille @samuelcolvin I believe that the introduction of explicitly setting __hash__ to None for non frozen BaseModels is a backwards incompatible breaking change for models which inherit from a common BaseModel implementation that defines __hash__ and __eq__.

Minimal reproduction:

import pydantic                                                                 
from pydantic import BaseModel                                                  
                                                                                
print(f"Version: {pydantic.version.VERSION}")                                   
                                                                                
class HashableModel(BaseModel):                                                 
    x: int                                                                      
                                                                                
    class Config:                                                               
        allow_mutation = False                                                  
                                                                                
    def __hash__(self):                                                         
        return hash((type(self),) + tuple(self.__dict__.values()))              
                                                                                
    def __eq__(self, other):                                                    
        return self.__hash__() == other.__hash__()                              
                                                                                
class ChildModel(HashableModel):                                                
    y: int                                                                      
                                                                                
# This is fine                                                                  
test = {HashableModel(x=4): HashableModel(x=2)}                                 
print(test)                                                                     
                                                                                
# This will fail on 1.8                                                                                                                                                                                                                             
test = {ChildModel(x=4, y=2): ChildModel(x=2, y=2)}                             
print(test)

On 1.7.3:

Version: 1.7.3
{HashableModel(x=4): HashableModel(x=2)}
{ChildModel(x=4, y=2): ChildModel(x=2, y=2)}

On 1.8:

Version: 1.8
{HashableModel(x=4): HashableModel(x=2)}
Traceback (most recent call last):
  File "repro.py", line 26, in <module>
    test = {ChildModel(x=4, y=2): ChildModel(x=2, y=2)}
TypeError: unhashable type: 'ChildModel'

A temporary workaround that is both forwards and backwards compatible is to set frozen explicitly to True on the superclass:

class Config:                                                               
    allow_mutation = False  
    frozen = True

Looking at the patch I'm not sure if there is a simple way to fix this, but I imagine this might hit some folks who have previously defined a common ImmutableBaseModel and are using that instead of BaseModel various places.

jolynch added a commit to Netflix-Skunkworks/service-capacity-modeling that referenced this issue Mar 1, 2021
@samuelcolvin
Copy link
Member

Is this fixed by #2423?

@jolynch
Copy link

jolynch commented Mar 1, 2021

Yes it is fixed by that patch, verified with:

$ python3 -m venv venv_test
$ venv_test/bin/pip install wheel
$ venv_test/bin/pip install git+https://github.com/samuelcolvin/pydantic.git@f9fe4aa086e7458f3c3339c408b3215f34e6ca18
$ venv_test/bin/python repro.py
Version: 1.8
{HashableModel(x=4): HashableModel(x=2)}
{ChildModel(x=4, y=2): ChildModel(x=2, y=2)}

Thanks and apologies for not seeing the new patch!

@ekampf
Copy link

ekampf commented Mar 2, 2021

@samuelcolvin any chance you can push a 1.8.1 with that fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants