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

Allow integers in TypeAlias names. #8488

Merged
merged 6 commits into from
Mar 28, 2023
Merged

Conversation

sodul
Copy link
Contributor

@sodul sodul commented Mar 23, 2023

Type of Changes

Type
βœ“ πŸ› Bug fix
βœ“ πŸ“œ Docs

Description

Updated the pattern to allow integers and updated the documentation to reflect the rule behavior: Good2Name, GoodName2, _1BadName.

towncrier create 8485.false_positive

Closes #8485.

Added checks and updated documentation to reflect the rule behavior: Good2Name, GoodName2, _1BadName.
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.

Could you add a news fragment as well? Rest LGTM! Thanks for picking this up!

| | ``typevars``. Note that ``TopName`` is allowed but | |
| | ``TTopName`` isn't. | |
| ``typealias`` | ``GoodName``, ``_GoodName``, ``IPAddressType``, | ``BadNameT``, ``badName``, ``TBadName``, ``TypeBadName`` |
| | 'GoodName2 and other PascalCase variants that don't | |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| | 'GoodName2 and other PascalCase variants that don't | |
| | ``GoodName2`` and other PascalCase variants that don't | |

@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Merging #8488 (fa53728) into main (f95b5b1) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8488   +/-   ##
=======================================
  Coverage   95.92%   95.92%           
=======================================
  Files         174      174           
  Lines       18361    18361           
=======================================
  Hits        17612    17612           
  Misses        749      749           
Impacted Files Coverage Ξ”
pylint/checkers/base/name_checker/checker.py 98.58% <ΓΈ> (ΓΈ)

@github-actions

This comment has been minimized.

@sodul sodul requested a review from DanielNoord March 23, 2023 23:57
@github-actions

This comment has been minimized.

@sodul sodul requested a review from DanielNoord March 25, 2023 00:05
@sodul
Copy link
Contributor Author

sodul commented Mar 25, 2023

The documentation check seem to have failed for an unrelated issue:

sphinx-build -b linkcheck -q -d _build/doctrees -T -W --keep-going   -n . _build/linkcheck
[1299](https://github.com/PyCQA/pylint/actions/runs/4506164141/jobs/7938251958?pr=8488#step:7:1300)
/home/runner/work/pylint/pylint/doc/whatsnew/2/2.13/full.rst:524: WARNING: broken link: https://www.gnu.org/software/emacs/manual/html_node/elisp/File-Locks.html (HTTPSConnectionPool(host='www.gnu.org', port=443): Max retries exceeded with url: /software/emacs/manual/html_node/elisp/File-Locks.html (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f9843d3d790>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')))

@sodul
Copy link
Contributor Author

sodul commented Mar 27, 2023

@DanielNoord I synced the branch with the latest main. I hoped this will get the last check to pass.

@github-actions

This comment has been minimized.

@Pierre-Sassoulas Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Mar 28, 2023
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.17.2 milestone Mar 28, 2023
@DanielNoord DanielNoord enabled auto-merge (squash) March 28, 2023 20:32
@@ -0,0 +1,5 @@
``invalid-name`` now allows for integers in ``typealias`` names:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
``invalid-name`` now allows for integers in ``typealias`` names:
``invalid-name`` now allows for integers in ``typealias`` names:

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't make commits to the branch so you'll need to do this yourself

@DanielNoord DanielNoord merged commit 43eb61e into pylint-dev:main Mar 28, 2023
github-actions bot pushed a commit that referenced this pull request Mar 28, 2023
@github-actions
Copy link
Contributor

πŸ€– Effect of this PR on checked open source code: πŸ€–

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

  1. unnecessary-lambda:
    Lambda may not be necessary
    https://github.com/pandas-dev/pandas/blob/10000db023208c1db0bba6a7d819bfe87dc49908/pandas/tests/resample/test_resampler_grouper.py#L294

This comment was generated for commit fa53728

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invalid-name check for TypeAlias should allow for digits in names
3 participants