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 support for binary union types - Python 3.10 #1977

Merged
merged 9 commits into from Jan 30, 2023

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Jan 27, 2023

Description

Add basic inference support for runtime binary or unions added in Python 3.10.
Not completely sure this is 100% the right approach for it, but it works. We can always iterate on it later if we want / need to.

At runtime int | None is represented as an instance of types.UnionType.

Refs pylint-dev/pylint#8119

--
After this is merged, I think it would make sense to do a new release so we can include it in pylint 2.16.0.

@cdce8p cdce8p added this to the 2.13.4 milestone Jan 27, 2023
@cdce8p cdce8p added backport maintenance/2.13.x pylint-tested PRs that don't cause major regressions with pylint labels Jan 27, 2023
@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Merging #1977 (25d5a01) into main (e31c9c4) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1977      +/-   ##
==========================================
+ Coverage   92.68%   92.70%   +0.01%     
==========================================
  Files          94       94              
  Lines       10891    10920      +29     
==========================================
+ Hits        10094    10123      +29     
  Misses        797      797              
Flag Coverage Δ
linux 92.46% <100.00%> (+0.02%) ⬆️
pypy 88.43% <43.33%> (-0.13%) ⬇️
windows 92.38% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
astroid/bases.py 88.53% <100.00%> (+0.54%) ⬆️
astroid/inference.py 94.58% <100.00%> (+0.04%) ⬆️
astroid/raw_building.py 92.49% <100.00%> (+0.18%) ⬆️

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.

I think this can go in 2.16.

astroid/inference.py Show resolved Hide resolved
@Pierre-Sassoulas
Copy link
Member

But shouldn't it be in astroid 2.14 ?

@cdce8p
Copy link
Member Author

cdce8p commented Jan 27, 2023

But shouldn't it be in astroid 2.14 ?

It certainly could. Moved the changelog entry.
We would then only need to decide if we want to release 2.13.4 with the typing_extensions.TypeVar fix or just include it with 2.14.

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

One minor nit. Rest LGTM!

astroid/bases.py Outdated Show resolved Hide resolved
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.

We should probably cover callable, pytype, display_type, bool_value and __str__ if only to prevent regression later, right ?

@cdce8p
Copy link
Member Author

cdce8p commented Jan 30, 2023

We should probably cover callable, pytype, display_type, bool_value and __str__ if only to prevent regression later, right ?

Added some more asserts. Although it might be that we need to go back and change some of them later. I don't know every detail of how the new class will be used by astroid and pylint.

@cdce8p
Copy link
Member Author

cdce8p commented Jan 30, 2023

The isort issue will be fixed by #1989

@Pierre-Sassoulas
Copy link
Member

👍

@cdce8p
Copy link
Member Author

cdce8p commented Jan 30, 2023

👍

@Pierre-Sassoulas Think you actually need to approve it since you requested changes earlier. Or I could just bypass the branch protection in this case 😄

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.

My bad 😄

@Pierre-Sassoulas Pierre-Sassoulas merged commit 156db06 into pylint-dev:main Jan 30, 2023
@cdce8p cdce8p deleted the binary-or branch January 30, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inference pylint-tested PRs that don't cause major regressions with pylint python 3.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants