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

New max-positional-arguments in new pylint version - failing CI-linter #1147

Closed
GernotMaier opened this issue Sep 23, 2024 · 7 comments · Fixed by #1151
Closed

New max-positional-arguments in new pylint version - failing CI-linter #1147

GernotMaier opened this issue Sep 23, 2024 · 7 comments · Fixed by #1151

Comments

@GernotMaier
Copy link
Contributor

The nightly CI-Linter run failed due to a pylint update with a new rule on max-positional-arguments (see here and here).

What do we do?

Simplest solution: move to newest version and add

# Maximum number of positional arguments for function / method (default=5.)
max-positional-arguments = 15

to the pypproject.toml file? This means we have to update everywhere pylint (otherwise you will get errors with an older version of pylint like E0015: Unrecognized option found: max-positional-arguments (unrecognized-option)).

Although I am not convinced that this rule should be so stringent, my suggestion would be to fix this and restrict the number of positional arguments to 5. In the long term it is easier when we follow default values.

@orelgueta
Copy link
Contributor

If I understand this requirement correctly, there is no limit for named arguments, right?
That is, if we have

func(
        arg1=arg1,
        arg2=arg2,
        arg3=arg3,
        arg4=arg4,
        arg5=arg5,
        arg6=arg6,
        arg7=arg7,
    )

That's allowed, right?

If so, I would vote for fixing all occurrences and just provide named arguments everywhere (which we generally try to do anyway).

@GernotMaier
Copy link
Contributor Author

There is a limit on the maximum number of arguments, which we have increased a lot in pyproject.toml (I think 15 is excessive):

# Maximum number of arguments for function / method (default=5).
max-args = 15

But isn't the way we are using it the right way to indicate which arguments are required and which are optional?

@orelgueta
Copy link
Contributor

But positional arguments are not necessarily related to required or optional, no?
You can provide required arguments out of order, as long as they are named.

I agree 15 is excessive, but 5 is too low. We can reduce the limit to 10 perhaps, but take this opportunity to also avoid using positional arguments in general (unless it is just 1-2 arguments).

@GernotMaier
Copy link
Contributor Author

I think I don’t know how to indicate requirement arguments which do not appear as positional arguments. How would one do that?

@orelgueta
Copy link
Contributor

Not at the definition side, only when calling the function. If at the definition side we have functions with more than 5 positional arguments, then I agree it is too much. When calling the function we can avoid it though and then I don't think it's a problem.

@GernotMaier
Copy link
Contributor Author

The issue is that I don’t exactly understand the pylint complaint. E.g. in

It looks to me like 3 positional arguments and 6 others. Pylint are complaining about 9 positional arguments.

@orelgueta
Copy link
Contributor

Yes, it probably doesn't like the fact we listed the arguments. But I wouldn't really call them positional ones if they have a default value and I much prefer that to *args, **kargs. I therefore vote to just increase the limit to 15.

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 a pull request may close this issue.

2 participants