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

Add __hash__ to PartialDate #53

Merged
merged 2 commits into from
Nov 13, 2022
Merged

Conversation

amCap1712
Copy link
Collaborator

In Python 3, the data model was changed to not provide a __hash__ method if __eq__ is overriden in a class. PartialDate defines __eq__ but not __hash__ and is thus unhashable. To quote the docs,

A class that overrides __eq__() and does not define __hash__() will have its __hash__() implicitly set to None. When the __hash__() method of a class is None, instances of the class will raise an appropriate TypeError when a program attempts to retrieve their hash value, and will also be correctly identified as unhashable when checking isinstance(obj, collections.abc.Hashable).

(from https://docs.python.org/3.7/reference/datamodel.html#object.__hash__)

We store PartialDate's in a set in SIR so it would be nice to make it hashable.

With SQLAlchemy warnings enabled, SQLAlchemy 1.4 gives the following warning.

MovedIn20Warning: The ``declarative_base()`` function is now available as sqlalchemy.orm.declarative_base(). (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Base = declarative_base()

The rationale of the move is explained here: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#declarative-becomes-a-first-class-api

mbdata's requirements.txt does not specify any fixed SQLAlchemy version
so we need to account for versions before 1.4 as well where this import
does not exist.
In Python 3, the data model was changed to not provide a __hash__ method
if __eq__ is overriden in a class. PartialDate defines __eq__ but not
__hash__ and is thus unhashable. To quote the docs,

> A class that overrides __eq__() and does not define __hash__() will have
its __hash__() implicitly set to None. When the __hash__() method of a
class is None, instances of the class will raise an appropriate TypeError
when a program attempts to retrieve their hash value, and will also be
correctly identified as unhashable when checking isinstance(obj, collections.abc.Hashable).

(from https://docs.python.org/3.7/reference/datamodel.html#object.__hash__)

We store PartialDate's in a set in [SIR](https://github.com/metabrainz/sir) so it would be nice to make it hashable.
@amCap1712
Copy link
Collaborator Author

Includes changes from #52 but not necessarily dependent on it.

amCap1712 added a commit to metabrainz/sir that referenced this pull request Oct 16, 2022
amCap1712 added a commit to metabrainz/sir that referenced this pull request Oct 17, 2022
amCap1712 added a commit to metabrainz/sir that referenced this pull request Oct 28, 2022
amCap1712 added a commit to metabrainz/sir that referenced this pull request Oct 28, 2022
@lalinsky lalinsky merged commit 58ee6dc into acoustid:main Nov 13, 2022
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.

2 participants