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 Vector.zero as a class-level attribute #168

Merged
merged 1 commit into from
Oct 26, 2021
Merged

Add Vector.zero as a class-level attribute #168

merged 1 commit into from
Oct 26, 2021

Conversation

nbraud
Copy link
Collaborator

@nbraud nbraud commented Jun 1, 2019

No description provided.

@nbraud
Copy link
Collaborator Author

nbraud commented Jun 2, 2019

Looks like the version of dataclasses in PyPI, which we use with Py3.6, doesn't handle properly typing.ClassVar.

Edit: This is a known-issue: ericvsmith/dataclasses#143

@AstraLuma
Copy link
Member

I guess comment out the annotation, with a link to that bug report.

@nbraud
Copy link
Collaborator Author

nbraud commented Jun 2, 2019

@astronouth7303 Without the annotation, dataclasses assumes it is an instance-level attribute and explodes.

I ported the fix from CPython to PyPI's dataclasses: ericvsmith/dataclasses#145

@nbraud
Copy link
Collaborator Author

nbraud commented Jun 2, 2019

Update: There already was a PR that backported that fix and another: ericvsmith/dataclasses#140
Unfortunately, it's been sitting there since December, so we can't expect upstream to merge that and cut a release any time soon. :(

@AstraLuma
Copy link
Member

AstraLuma commented Jun 2, 2019

Ok, so what parts of dataclasses exactly chokes on this?

If we can't find a workaround, lets turn this into a factory method, or just leave it out altogether.

@nbraud
Copy link
Collaborator Author

nbraud commented Jun 2, 2019

Yeah, a factory method would work, but I'm willing to let this sit until there's a fixed version of dataclasses (or until we decide only to support Py3.7 and 3.8, whichever comes first) since it's really not high priority.

@nbraud
Copy link
Collaborator Author

nbraud commented Jun 6, 2019

bors try

bors bot added a commit that referenced this pull request Jun 6, 2019
@bors
Copy link
Contributor

bors bot commented Jun 6, 2019

try

Timed out

@nbraud
Copy link
Collaborator Author

nbraud commented Jun 6, 2019

One macOS task seems to not have run. Rude :(

In any case, that confirms the testsuite passes now that #167 has been merged, though I would hold this up until either dataclasses is fixed or we drop Python 3.6 support.

@nbraud nbraud added blocked Blocked by another issue, potentially external to the project. enhancement labels Jun 6, 2019
This was referenced Nov 8, 2019
bors bot added a commit that referenced this pull request Dec 19, 2019
190: Drop compatiblity with Python 3.6 r=astronouth7303 a=nbraud

We only aim to support the last 2 major versions of Python (currently 3.7 and 3.8), and the `dataclasses` backport for Python 3.6 is unmaintained and broken (see discussion in #184).

This should allow us to unblock #168 

Co-authored-by: Nicolas Braud-Santoni <nicolas@braud-santoni.eu>
@pathunstrom pathunstrom changed the base branch from master to canon June 27, 2020 11:02
bors bot added a commit that referenced this pull request Oct 23, 2021
190: Drop compatiblity with Python 3.6 r=astraluma a=nbraud

We only aim to support the last 2 major versions of Python (currently 3.7 and 3.8), and the `dataclasses` backport for Python 3.6 is unmaintained and broken (see discussion in #184).

This should allow us to unblock #168 

Co-authored-by: Nicolas Braud-Santoni <nicolas@braud-santoni.eu>
bors bot added a commit that referenced this pull request Oct 23, 2021
190: Drop compatiblity with Python 3.6 r=astraluma a=nbraud

We only aim to support the last 2 major versions of Python (currently 3.7 and 3.8), and the `dataclasses` backport for Python 3.6 is unmaintained and broken (see discussion in #184).

This should allow us to unblock #168 

Co-authored-by: Nicolas Braud-Santoni <nicolas@braud-santoni.eu>
Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
bors bot added a commit that referenced this pull request Oct 23, 2021
190: Drop compatiblity with Python 3.6 r=astraluma a=nbraud

We only aim to support the last 2 major versions of Python (currently 3.7 and 3.8), and the `dataclasses` backport for Python 3.6 is unmaintained and broken (see discussion in #184).

This should allow us to unblock #168 

Co-authored-by: Nicolas Braud-Santoni <nicolas@braud-santoni.eu>
@nbraud nbraud requested a review from AstraLuma October 23, 2021 16:48
@nbraud nbraud removed the blocked Blocked by another issue, potentially external to the project. label Oct 23, 2021
@nbraud
Copy link
Collaborator Author

nbraud commented Oct 26, 2021

@AstraLuma Rebased to fix the merge conflict, should be finally good to go.

@AstraLuma
Copy link
Member

This breaks on 3.6, right?

@nbraud
Copy link
Collaborator Author

nbraud commented Oct 26, 2021

This breaks on 3.6, right?

Not sure what you mean there, but:

  • I merged Drop compatiblity with Python 3.6 #190 so 3.6 support is gone; presumably, if it was OK to go 2 years ago, it was still OK now.
  • Even if we were to revert that, this code is fine on Py3.6; however, the unmaintained dataclasses compatibility shim for Py3.6 has (or had?) bugs that made it explode on ClassVar annotations, so for instance dataclasses.replace would choke on a Vector (IIRC throw an exception)

Copy link
Member

@AstraLuma AstraLuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

@bors bors bot merged commit 273df90 into ppb:canon Oct 26, 2021
@nbraud nbraud deleted the zero branch October 26, 2021 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants