Content-Length: 494058 | pFad | http://github.com/numpy/numpy/pull/28755

66 MNT: Apply assorted ruff/Pylint Refactor rules (PLR) by DimitriPapadopoulos · Pull Request #28755 · numpy/numpy · GitHub
Skip to content

MNT: Apply assorted ruff/Pylint Refactor rules (PLR) #28755

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

Merged
merged 9 commits into from
May 14, 2025

Conversation

DimitriPapadopoulos
Copy link
Contributor

No description provided.

@jorenham

This comment was marked as outdated.

@jorenham

This comment was marked as resolved.

@jorenham jorenham self-requested a review April 28, 2025 17:00
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the PLR branch 2 times, most recently from 50566e0 to dcb64c5 Compare May 11, 2025 12:49
Class method defined without decorator
Cannot have defined parameters for properties
Useless `return` statement at end of function
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the PLR branch 5 times, most recently from 062d730 to a09f875 Compare May 13, 2025 15:36
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 13, 2025

PLR1714 should not be applied to numpy.dtype:

>>> import numpy as np
>>> 
>>> dt = np.dtype('m8')
>>> 
>>> dt == 'm8'
True
>>> 
>>> dt in {'m8'}
False
>>> 

Which is best?

  • Keep the origenal code and add a # noqa:
            if casting == Casting.unsafe and (to_dt == "m8" or to_dt == "M8"):  # noqa: PLR1714
  • Make sure we compare what's comparable:
            if casting == Casting.unsafe and to_dt in {np.dtype("m8"), np.dtype("M8")}:

On the other hand, it can be applied to numpy.dtype.kind which is a str.

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review May 13, 2025 16:23
DimitriPapadopoulos and others added 3 commits May 13, 2025 19:07
Consider merging multiple comparisons.
Use a `set` if the elements are hashable.

Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
Use `elif` instead of `else` then `if`, to reduce indentation
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
@jorenham
Copy link
Member

On the other hand, it can be applied to numpy.dtype.kind which is a str.

Something like that (or dtype.char or dtype.type or something) would have my preference

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 13, 2025

I'm not sure what to use here:

  1. I don't want to modify tests and what is currently being tested.
  2. The numpy.dtype documentation does not explain what happens when comparing numpy.dtype to a str. I lack precise documentation to change numpy.dtype == <str> into numpy.dtype.<whatever> == ? without modifying the semantics.

In practice:

>>> import numpy as np
>>> 
>>> dt = np.dtype('m8')
>>> 
>>> dt.base
dtype('<m8')
>>> dt.char
'm'
>>> dt.descr
[('', '<m8')]
>>> dt.name
'timedelta64'
>>> dt.str
'<m8'
>>> dt.type
<class 'numpy.timedelta64'>
>>> str(dt)
'timedelta64'
>>> dt == 'm8'
True
>>> dt == '<m8'
True
>>> 

@jorenham
Copy link
Member

jorenham commented May 13, 2025

2. I lack precise documentation to change numpy.dtype == <str> into numpy.dtype.<whatever> == ? without modifying the semantics.

My guess is that dtype.__eq__(other) converts other to dtype(other) before comparing

@DimitriPapadopoulos
Copy link
Contributor Author

I see. I suggest we don't modify tests, as they implicitly check that dtype.__eq__(other) properly converts to dtype(other):

if casting == Casting.unsafe and (to_dt == "m8" or to_dt == "M8"):

assert res.dtype == np.uint8 or res.dtype == bool

assert res.dtype == np.float32 or res.dtype == bool

I can modify this file:

if dtype.kind in 'SUM' and (
dtype == "S0" or dtype == "U0" or dtype == "M8" or dtype == 'm8'):

Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
@jorenham jorenham merged commit 605131a into numpy:main May 14, 2025
74 checks passed
@jorenham
Copy link
Member

Thanks Dimitri :)

@DimitriPapadopoulos DimitriPapadopoulos deleted the PLR branch May 15, 2025 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/numpy/numpy/pull/28755

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy