Skip to content
This repository was archived by the owner on Mar 8, 2018. It is now read-only.

Fix for python-attrs/attrs#253 breaking change #13

Open
wants to merge 1 commit into
base: feature/hash-by-id
Choose a base branch
from
Open

Fix for python-attrs/attrs#253 breaking change #13

wants to merge 1 commit into from

Conversation

mjdunn
Copy link

@mjdunn mjdunn commented Jan 23, 2018

As reported in #12, python-attrs/attrs#253 currently breaks compatibility with attrs 17.3+. This commit resets "this" attributes back onto class bodies, preserving functionality.

This commit passes all test_attrs_sqlalchemy tests.

@rouge8
Copy link
Contributor

rouge8 commented Jan 23, 2018

Thanks for this. Internally, we've stopped using attrs_sqlalchemy in new code. #10 realized that using attrs for hashing was a bad idea. Using attrs for __eq__ is also not great and we should let SQLAlchemy do that itself based on identity. At that point, all attrs_sqlalchemy provides is a nice __repr__ and there's probably much nicer ways to do that than pulling in attrs.

I'll leave this open for now.

@rouge8
Copy link
Contributor

rouge8 commented Mar 7, 2018

Hi @mjdunn, we're going ahead and deprecating attrs_sqlalchemy and do not recommend using it going foward (#14).

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

Successfully merging this pull request may close these issues.

2 participants