MAINT: Start applying pycodestyle error rules (E) #27308

DimitriPapadopoulos · 2024-08-29T10:23:40Z

In preparation for #24994.

charris · 2024-08-30T13:47:31Z

Needs rebase.

charris · 2024-08-30T13:51:44Z

Might want tag ignore some of the long lines.

charris · 2024-09-04T12:56:01Z

@DimitriPapadopoulos Are you intentionally keeping this a draft?

DimitriPapadopoulos · 2024-09-04T14:17:27Z

I need to apply more E pycodestyle rules and thought you would prefer a single PR. Actually I prefer smaller changes, so I will remove the Draft label soon and leave other changes for later.

charris · 2024-09-05T19:15:24Z

doc/source/conf.py

-  ('user/index', 'numpy-user.tex', 'NumPy User Guide',
-   _stdauthor, 'manual'),
+    ('reference/index', 'numpy-ref.tex', 'NumPy Reference',
+    _stdauthor, 'manual'),


Shouldn't the continuation have an extra space?

charris · 2024-09-05T19:19:18Z

numpy/_core/_dtype.py

            return "'%sU%d'" % (byteorder, dtype.itemsize / 4)

-    elif dtype.type == str:
+    elif dtype.type is str:


Hmm, this might change behavior. @seberg Thoughts?

If this was dtype == then yes, as is. This is fine, although it is a bit funny to not use it above as well.

charris · 2024-09-05T19:23:25Z

numpy/_core/code_generators/generate_umath.py

+# name: [string of chars for which it is defined,
 #       string of characters using func interface,
 #       tuple of strings giving funcs for data,
 #       (in, out), or (instr, outstr) giving the signature as character codes,


Might add an extra space here even if a comment.

charris · 2024-09-05T19:27:09Z

numpy/_core/tests/test_casting_unittests.py

                            cast._resolve_descriptors((from_dt, to_dt)))
-                    assert(type(from_res) == from_Dt)
-                    assert(type(to_res) == to_Dt)
+                    assert type(from_res) is from_Dt


Might want to check that this is correct.

charris · 2024-09-05T19:32:32Z

numpy/_core/tests/test_datetime.py

+                         np.datetime64('NaT', '[D]'),
+                         np.timedelta64(3, '[D]'),
+                         np.timedelta64(11, '[h]'),
+                         np.timedelta64(3*24 - 11, '[h]'))]:


Hmm, I think here and below the contents are over indented. Might be a better way to write these, maybe put the opening [ on the preceding line and dispense with the line continuation?

charris · 2024-09-05T19:37:46Z

numpy/_core/tests/test_numeric.py

        kwargs = {'fill_value': ''} if likefunc == np.full_like else {}
        result = likefunc(b, dtype=dtype, **kwargs)
-        if dtype == str:
+        if dtype is str:


Might be worth a check.

charris · 2024-09-05T19:46:14Z

numpy/_core/tests/test_shape_base.py

+                    [one],
+                    [three],
+                    [four]
                ]),


This nesting here is difficult to understand, could use a reformatting.

charris · 2024-09-05T19:47:39Z

numpy/_core/tests/test_simd.py

    def test_tobits(self):
-        data2bits = lambda data: sum([int(x != 0) << i for i, x in enumerate(data, 0)])
+        def data2bits(data):
+            return sum([int(x != 0) << i for i, x in enumerate(data, 0)])


I'm a bit surprised a blank line wasn't added here.

These are automated fixes by ruff, and pycodestyle doesn't have anything to say about it at this point. I was thinking of handling this error in an upcoming PR that would handle the relevant E pycodestyle rule.

charris · 2024-09-05T19:48:51Z

numpy/_core/tests/test_simd.py


-        data_sqrt = self.load([math.sqrt(x) for x in data]) # load to truncate precision
+        data_sqrt = self.load([math.sqrt(x)  # load to truncate precision
+                               for x in data])


Would be nicer to move the trailing comment to the preceding line.

charris · 2024-09-05T20:02:27Z

numpy/_typing/_array_like.py

 class _SupportsArray(Protocol[_DType_co]):
-    def __array__(self) -> ndarray[Any, _DType_co]: ...
+    def __array__(self) -> ndarray[Any, _DType_co]:
+        ...


I think all these typing fixes should be avoided. @jorenham Thoughts?

See #27308 (comment)

What can I say? I'm just enforcing pycodestyle rules, not my choice. Unlike ruff, pycodestyle appears to be very opinionated, in the sense that errors cannot be silenced with noqa.

# fmt: skip maybe?

ruff also includes pycodestyle btw: https://docs.astral.sh/ruff/rules/#pycodestyle-e-w

charris · 2024-09-05T20:04:34Z

numpy/f2py/_src_pyf.py


-    substr = list_re.sub(listrepl, substr) # convert all lists to named templates
+    substr = list_re.sub(listrepl, substr)  # convert all lists to named templates
                                           # newnames are constructed as needed


Might want to line this up. Maybe better, put the comments on the preceding line. I hate trailing comments :)

charris · 2024-09-05T20:05:50Z

numpy/f2py/_src_pyf.py

    return writestr

+
 include_src_re = re.compile(r"(\n|\A)\s*include\s*['\"](?P<name>[\w\d./\\]+\.src)['\"]", re.I)


This line seems long ...

charris · 2024-09-05T20:15:10Z

numpy/linalg/tests/test_linalg.py

        for mat in self.rshft_all:
            tz(mat.astype(dt))
-            if dt != object:
+            if dt != object:  # noqa: E721


I'm curious why dt is not object didn't work here.

I am considering moving this commit to a PR of its own, for a separate discussion.

charris · 2024-09-05T20:18:02Z

numpy/ma/core.py

        mask = (
-            f(getmaskarray(a), np.ones(np.shape(v), dtype=bool), mode=mode)
-          | f(np.ones(np.shape(a), dtype=bool), getmaskarray(v), mode=mode)
+            f(getmaskarray(a), np.ones(np.shape(v), dtype=bool), mode=mode) |


IIRC, Python changed the style to prefer putting the operator in front. Don't recall if that was for Python or C code.

Indeed, see W503 / W504.

To be honest, I don't care about formatting, just about silencing pycodestyle. I'd rather let a formatter do the formatting for me. However, I understand black or ruff don't work well on numerical code.

I'm happy with leaving the change here, just reminded of the preference change.

charris · 2024-09-05T20:27:33Z

numpy/typing/tests/data/pass/ndarray_misc.py

-class SubClass(npt.NDArray[np.float64]): ...
+class SubClass(npt.NDArray[np.float64]):
+    ...
+


Again the typing.

charris · 2024-09-05T20:33:09Z

That was no fun, it would be better to keep the PR size down, and maybe do fewer style changes together. Going through a bunch of lines that add spaces in comments or blank lines is pretty easy, but mixing them in with more complicated fixes slows things down.

jorenham · 2024-09-05T20:42:49Z

For the static typing-related changes I agree with @charris that current "inlined ellipsis style", i.e. : ..., is conventional.
I believe that ruff format always enforces this for (at least) typing.Protocol, and always does so in .pyi stubs.

charris · 2024-09-11T15:24:05Z

@DimitriPapadopoulos What are your plans here? With a few cleanups we could put this in.

charris · 2024-09-25T14:25:32Z

@DimitriPapadopoulos We will be branching 2.2 in six weeks or so. It would be good to get these cleanups in before that happens.

DimitriPapadopoulos · 2024-09-27T07:50:27Z

@charris I am afraid I unintentionally deleted this branch 😳 That's not necessarily a big deal because I am not certain any more that fixing Pycodestyle issues in the whole codebase is the best strategy to move to the ruff linter. Perhaps we can continue the discussion about the best strategy in #24994.

DimitriPapadopoulos · 2024-09-27T13:46:51Z

Superseded by #27300.

github-actions bot added the 03 - Maintenance label Aug 29, 2024

DimitriPapadopoulos force-pushed the E branch 7 times, most recently from dbff374 to 793890f Compare August 29, 2024 12:44

DimitriPapadopoulos mentioned this pull request Aug 29, 2024

CI: pycodestyle → ruff #27300

Merged

DimitriPapadopoulos force-pushed the E branch 4 times, most recently from 82c7da1 to 23086e4 Compare August 29, 2024 13:10

DimitriPapadopoulos mentioned this pull request Aug 29, 2024

Linting Package Replacement: Pycodestyle to Ruff #24994

Closed

DimitriPapadopoulos force-pushed the E branch from 23086e4 to 3cb2490 Compare August 29, 2024 15:01

DimitriPapadopoulos changed the title ~~MAINT: Apply ruff/pycodestyle error rules (E)~~ MAINT: Apply pycodestyle error rules (E) Aug 29, 2024

DimitriPapadopoulos force-pushed the E branch 6 times, most recently from b177186 to 8d8ae15 Compare August 30, 2024 13:22

DimitriPapadopoulos force-pushed the E branch 6 times, most recently from 557a08f to b04b862 Compare September 1, 2024 19:04

DimitriPapadopoulos force-pushed the E branch 2 times, most recently from 189eb97 to 4a09789 Compare September 3, 2024 05:11

DimitriPapadopoulos marked this pull request as ready for review September 5, 2024 07:37

DimitriPapadopoulos changed the title ~~MAINT: Apply pycodestyle error rules (E)~~ MAINT: Start applying pycodestyle error rules (E) Sep 5, 2024

charris reviewed Sep 5, 2024

View reviewed changes

DimitriPapadopoulos closed this Sep 11, 2024

DimitriPapadopoulos force-pushed the E branch from 4a09789 to d6bfeb0 Compare September 11, 2024 17:44

		return writestr


		include_src_re = re.compile(r"(\n\|\A)\sinclude\s['\"](?P<name>[\w\d./\\]+\.src)['\"]", re.I)

Uh oh!

MAINT: Start applying pycodestyle error rules (E) #27308

MAINT: Start applying pycodestyle error rules (E) #27308

Uh oh!

Conversation

DimitriPapadopoulos commented Aug 29, 2024

Uh oh!

charris commented Aug 30, 2024

Uh oh!

charris commented Aug 30, 2024

Uh oh!

charris commented Sep 4, 2024

Uh oh!

DimitriPapadopoulos commented Sep 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charris Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charris commented Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jorenham commented Sep 5, 2024

Uh oh!

charris commented Sep 11, 2024

Uh oh!

charris commented Sep 25, 2024

Uh oh!

DimitriPapadopoulos commented Sep 27, 2024

Uh oh!

DimitriPapadopoulos commented Sep 27, 2024

Uh oh!

Reviewers

Assignees

charris Sep 5, 2024 •

edited

Loading

charris commented Sep 5, 2024 •

edited

Loading