chore: update py version in lint workflow to 3.13 #4450
Conversation
jomcgi
force-pushed
the
chore-update-ci-lint-pyversion
branch
2 times, most recently
from
ee82a5c to
8740573
Compare
jomcgi
force-pushed
the
chore-update-ci-lint-pyversion
branch
from
8740573 to
ac7329c
Compare
This upgrade was delayed due to an issue where pylint was unable to import collections.abc Update references to 3.13 now that the underlying issue has been resolved. Add pylint disable comments for existing violations
jomcgi
changed the title
| @@ -97,7 +97,7 @@ | |||
|
|
|||
|
|
|||
| class ZipkinExporter(SpanExporter): | |||
| def __init__( | |||
| def __init__( # pylint: disable=too-many-positional-arguments | |||
There was a problem hiding this comment.
Maybe we should disable this globally instead?
There was a problem hiding this comment.
I think it's a useful check - having a lot of positional arguments adds complexity & implicitly couples the order of arguments.
I might be missing something though as I'm less familiar with the spec / codebase.
Would the main benefit of disabling this globally be that we reduce the number of pylint disable comments?
There was a problem hiding this comment.
can we increase the values here? https://github.com/open-telemetry/opentelemetry-python/blob/main/.pylintrc#L455 I'm also concerned about adding pylint disable comments in every place
There was a problem hiding this comment.
I think there's merit to both approaches.
Increasing the max value globally would reduce pylint disable comments, but keeping the check and adding disable comments where necessary flags this for future contributors to consider.
For new implementations, we could also follow the Pylint docs suggestion of using * to enforce keyword-only arguments:
# Instead of:
def method(self, a, b, c, d, e): # [too-many-positional-arguments]
pass
# Consider:
def method(self, a, b, c, d, *, e=False):
passAs I'm relatively new to this project, those of you with more context may have better insight into which approach aligns best with the project's conventions.
Description
This upgrade was delayed due to an issue where pylint was unable to import collections.abc
pylint-dev/pylint#10112
Update references to 3.13 now that the underlying issue has been resolved.
Fixes #4366
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Contrib Repo Change?
Checklist: