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

Fix issues with new typing Union syntax (Py310) #8122

Merged
merged 2 commits into from Feb 1, 2023

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Jan 27, 2023

Description

Add regression tests for new typing Union syntax.
Bump astroid to 2.13.4.

Requires pylint-dev/astroid#1977
Closes #8119

@cdce8p cdce8p added Blocker 🙅 Blocks the next release Blocked 🚧 Blocked by a particular issue python 3.10 Needs astroid update Needs an astroid update (probably a release too) before being mergable backport maintenance/2.16.x labels Jan 27, 2023
@cdce8p cdce8p added this to the 2.16.0 milestone Jan 27, 2023
@cdce8p cdce8p removed the Blocked 🚧 Blocked by a particular issue label Jan 31, 2023
@cdce8p cdce8p marked this pull request as ready for review January 31, 2023 15:20
@Pierre-Sassoulas
Copy link
Member

It seems there's been a core dump two times in a row in the 3.7 tests and once in the 3.8 tests ? I wonder if this what happen in case of timeout now or if this is a genuine problem with astroid 2.14 ?

@cdce8p
Copy link
Member Author

cdce8p commented Jan 31, 2023

It seems there's been a core dump two times in a row in the 3.7 tests and once in the 3.8 tests ? I wonder if this what happen in case of timeout now or if this is a genuine problem with astroid 2.14 ?

It's the recursion issue from pylint-dev/astroid#1982 again, sadly. I wouldn't consider astroid broken though so we don't need to yank the releases. However, it might make sense to revert the change. What do you think?

It's a bit of work, but I could do it. With that I would also consider releasing 2.13.5 and 2.14.1.

@Pierre-Sassoulas
Copy link
Member

Hmm yeah, I have no idea how to fix this. Maybe a partial revert keeping the new added tests ? Also we don't need to maintain 2.13 anymore, we could only release 2.14.1.

@cdce8p
Copy link
Member Author

cdce8p commented Jan 31, 2023

Also we don't need to maintain 2.13 anymore, we could only release 2.14.1.

Yeah, also I kind of broke the last release of 2.13. Would be nice to fix it even if we don't "need" to.
I can try to do the backports manually, it should probably work then.

@Pierre-Sassoulas
Copy link
Member

Right, it branched 3 days ago, it's reasonable, there's not going to be any conflicts.

@cdce8p
Copy link
Member Author

cdce8p commented Jan 31, 2023

Did the revert, released both 2.13.5 + 2.14.1, and merged everything back into main. This should work now 🤞🏻

@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Merging #8122 (c70285d) into main (c70285d) will not change coverage.
The diff coverage is n/a.

❗ Current head c70285d differs from pull request most recent head 3f3fc1b. Consider uploading reports for the commit 3f3fc1b to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8122   +/-   ##
=======================================
  Coverage   95.53%   95.53%           
=======================================
  Files         177      177           
  Lines       18622    18622           
=======================================
  Hits        17791    17791           
  Misses        831      831           

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Great work with all the release ! I suppose it prove it's easier to do now than before 😄

@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2023

🤖 Effect of this PR on checked open source code: 🤖

Effect on black:
The following messages are no longer emitted:

  1. no-member:
    Instance of 'SyncManager' has no 'Lock' member; maybe 'RLock'?
    https://github.com/psf/black/blob/226cbf0226ee3bc26972357ba54c36409e9a84ae/src/black/concurrency.py#L151

This comment was generated for commit 3f3fc1b

@cdce8p cdce8p merged commit 1b5bba4 into pylint-dev:main Feb 1, 2023
@cdce8p cdce8p deleted the binary-or branch February 1, 2023 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker 🙅 Blocks the next release Needs astroid update Needs an astroid update (probably a release too) before being mergable python 3.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with new Union Syntax
2 participants