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 max-positional-arguments config #637

Closed
wants to merge 1 commit into from

Conversation

t-imamichi
Copy link
Collaborator

Summary

Pylint 3.3 introduced a new check too-many-positional-arguments.
https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/too-many-positional-arguments.html

This PR adds a config for the check.

Details and comments

@t-imamichi t-imamichi changed the title Add max-positional-args config Add max-positional-arguments config Oct 15, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 11342429555

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.857%

Totals Coverage Status
Change from base Build 10312432197: 0.0%
Covered Lines: 4446
Relevant Lines: 4788

💛 - Coveralls

@woodsp-ibm
Copy link
Member

On other apps we disabled the too many positional on the line it occurred. Any new code that was done with so many positional args, would get warned against so it could be improved - while setting the config this way it would just be accepted. I know in newer code we had tried to do better with some pos args and then using * to enforce keyword only. But yes it does have an issue with the older lint that is only supported in 3.8. For that one needs to add "unknown-option-value" to the list of lint errors in teh config that is ignored. Soon qiskit will drop support for 3.8 anyway and that then would be unnecessary.

@t-imamichi
Copy link
Collaborator Author

Thank you for your feedback, @woodsp-ibm. I then close this PR and review #635 so that we merge it soon.

@t-imamichi t-imamichi closed this Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants