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

False positive nested-min-max for nested lists #9307

Closed
udifuchs opened this issue Dec 15, 2023 · 4 comments · Fixed by #9323
Closed

False positive nested-min-max for nested lists #9307

udifuchs opened this issue Dec 15, 2023 · 4 comments · Fixed by #9323
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@udifuchs
Copy link
Contributor

Bug description

"""False positive `nested-min-max` for nested lists"""
a = [[1, 2, 3], [4, 5, 6]]
print(max(max(a)))  # Output: 6
print(max(*a))  # Output: [4, 5, 6]

Configuration

No response

Command used

pylint a.py

Pylint output

************* Module a
a.py:3:6: W3301: Do not use nested call of 'max'; it's possible to do 'max(*a)' instead (nested-min-max)

Expected behavior

Pylint gives wrong advice. If a is a nest list, max(max(a)) is not equivalent max(*a).

Pylint version

pylint 3.0.3
astroid 3.0.2
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0]

OS / Environment

No response

Additional dependencies

No response

@udifuchs udifuchs added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Dec 15, 2023
@jacobtylerwalls
Copy link
Member

I wonder if we can recommend itertools.chain.from_iterable() here instead. Thanks for the clear report.

@jacobtylerwalls jacobtylerwalls added Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Dec 16, 2023
@Pierre-Sassoulas
Copy link
Member

Isnt't max(max(a)) more elegant ? Should we consider this a false positive ?

@jacobtylerwalls
Copy link
Member

I haven't benchmarked itertools but I would assume it's better than iterating twice. chain.from_iterable is marginally uglier, but I would guess that when people prefer explicitness over performance they'd probably disable the whole check. Thoughts?

@Pierre-Sassoulas
Copy link
Member

Ha sure, sometime I don't remember or it's unclear what the goal of a check is. perfs and readability are sometime incompatible.

udifuchs added a commit to udifuchs/pylint that referenced this issue Dec 24, 2023
Nesting is useful for finding the maximum in a matrix.
Therefore, pylint allows nesting of the form:
max(max([[1, 2, 3], [4, 5, 6]]))

Closes pylint-dev#9307
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.0.4 milestone Dec 26, 2023
Pierre-Sassoulas added a commit that referenced this issue Dec 30, 2023
Nesting is useful for finding the maximum in a matrix.
Therefore, pylint allows nesting of the form:
max(max([[1, 2, 3], [4, 5, 6]]))

Closes #9307

Co-authored-by: Pierre Sassoulas <[email protected]>
github-actions bot pushed a commit that referenced this issue Dec 30, 2023
Nesting is useful for finding the maximum in a matrix.
Therefore, pylint allows nesting of the form:
max(max([[1, 2, 3], [4, 5, 6]]))

Closes #9307

Co-authored-by: Pierre Sassoulas <[email protected]>
(cherry picked from commit da13c74)
Pierre-Sassoulas pushed a commit that referenced this issue Dec 30, 2023
Nesting is useful for finding the maximum in a matrix.
Therefore, pylint allows nesting of the form:
max(max([[1, 2, 3], [4, 5, 6]]))

Closes #9307

Co-authored-by: Pierre Sassoulas <[email protected]>
(cherry picked from commit da13c74)

Co-authored-by: Udi Fuchs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants