STYLE ruff: enable PIE #51434

ch1booze · 2023-02-16T16:18:29Z

closes STYLE ruff: enable PIE #51407
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

MarcoGorelli

thanks for working on this!

MarcoGorelli · 2023-02-16T16:26:03Z

.pre-commit-config.yaml

@@ -19,6 +19,7 @@ repos:
    rev: v0.0.244
    hooks:
    -   id: ruff
+        args: [--fix, --exit-non-zero-on-fix]


can we not add this please? there's a few false positives which risk confusing contributors

Okay. Would remove it.

MarcoGorelli · 2023-02-16T16:26:16Z

pandas/core/series.py

@@ -836,8 +836,6 @@ def view(self, dtype: Dtype | None = None) -> Series:

    # ----------------------------------------------------------------------
    # NDArray Compat
-    _HANDLED_TYPES = (Index, ExtensionArray, np.ndarray)


why remove this?

The error was PIE794. The fix that was suggested was removing one of the instances.

ah you're right, it is indeed duplicated. thanks!

MarcoGorelli · 2023-02-16T16:26:26Z

pandas/io/stata.py

@@ -875,7 +875,6 @@ class StataMissingValue:
        float32_base = struct.pack("<i", int_value)

    float64_base: bytes = b"\x00\x00\x00\x00\x00\x00\xe0\x7f"
-    increment = struct.unpack("q", b"\x00\x00\x00\x00\x00\x01\x00\x00")[0]


why remove this?

The error was PIE794. The fix that was suggested was removing one of the instances.

thanks - I don't think we can remove it though

can you rename the first one to increment_32 and the second one to increment_64 please?

I have corrected this.

MarcoGorelli · 2023-02-16T17:16:49Z

doc/source/user_guide/style.ipynb

@@ -47,6 +47,7 @@
   "outputs": [],
   "source": [
    "import matplotlib.pyplot\n",
+    "\n",


why did this file change?

Happened after I formatted using black by running:

black .

in bash.
It seems like it would be better if I put the reasons behind my changes in the description. Would do that next time.

no worries - let's revert this, CI isn't running black-jupyter

if you run black via pre-commit run black --all-files then that'll match what's in CI

Removed the fix arguments.

ch1booze · 2023-02-16T18:01:56Z

@MarcoGorelli
There is a conflict here. How should it be resolved?

MarcoGorelli · 2023-02-16T19:06:39Z

@MarcoGorelli There is a conflict here. How should it be resolved?

hey - I'd suggest looking at https://opensource.com/article/20/4/git-merge-conflict for how to resolve conflicts

MarcoGorelli · 2023-02-16T19:50:04Z

looks like you managed, nice!

just need to revert changes to doc/source/user_guide/style.ipynb, if you do something like

git fetch upstream
git checkout upstream/main -- doc/source/user_guide/style.ipynb
git commit -a -m 'revert changes to doc/source/user_guide/style.ipynb'
git push origin HEAD

it should work

ch1booze · 2023-02-16T22:24:25Z

@MarcoGorelli
I think I am done with the requested changes barring any changes.

MarcoGorelli

Nice work, looks good to me pending green

ch1booze · 2023-02-17T06:00:45Z

Nice work, looks good to me pending green

@MarcoGorelli
Does that mean I could close the pull request? The reason for my asking is that I am also seeing a lot of errors popping up in the checks that are being carried out.
By the way, this would be my first open-source contribution, when completed. Thanks for the opportunity.

MarcoGorelli · 2023-02-17T08:55:26Z

pandas/io/formats/css.py

    CSS_EXPANSIONS = {
-        **{
+        {
            (f"border-{prop}" if prop else "border"): _border_expander(prop)
            for prop in ["", "top", "right", "bottom", "left"]
        },
-        **{
+        {
            f"border-{prop}": _side_expander(f"border-{{:s}}-{prop}")
            for prop in ["color", "style", "width"]
        },
-        **{
+        {
            "margin": _side_expander("margin-{:s}"),
            "padding": _side_expander("padding-{:s}"),
        },


looks like this one is causing some test failures

You probably want

CSS_EXPANSIONS = { **{ (f"border-{prop}" if prop else "border"): _border_expander(prop) for prop in ["", "top", "right", "bottom", "left"] }, **{ f"border-{prop}": _side_expander(f"border-{{:s}}-{prop}") for prop in ["color", "style", "width"] }, "margin": _side_expander("margin-{:s}"), "padding": _side_expander("padding-{:s}"), }

This is the PIE error that was generated.

MarcoGorelli · 2023-02-17T08:56:51Z

@MarcoGorelli Does that mean I could close the pull request? The reason for my asking is that I am also seeing a lot of errors popping up in the checks that are being carried out.

when it's ready, we will merge it - but first, the errors need fixing up

ch1booze · 2023-02-21T14:02:38Z

@MarcoGorelli Sorry, I have been away for a while. I have re-run and I still see the errors. Looking through to see where I might have gone wrong.

MarcoGorelli · 2023-02-21T14:08:07Z

pandas/tests/frame/methods/test_to_records.py

-            "column_dtypes": DictLike(**{"A": np.int8, "B": np.float32}),
+            "column_dtypes": DictLike({"A": np.int8, "B": np.float32}),


this isn't right, you want

"column_dtypes": DictLike(A=np.int8, B=np.float32),

you can run this test with

pytest pandas/tests/frame/methods/test_to_records.py

Running pytest command is giving:

ImportError while loading conftest 'C:\Users\Victor Momodu\Documents\Programming\Data Science\Code\pandas\pandas\conftest.py'. pandas\__init__.py:22: in <module> from pandas.compat import is_numpy_dev as _is_numpy_dev # pyright: ignore # noqa:F401 pandas\compat\__init__.py:25: in <module> from pandas.compat.numpy import ( pandas\compat\numpy\__init__.py:4: in <module> from pandas.util.version import Version pandas\util\__init__.py:2: in <module> from pandas.util._decorators import ( # noqa:F401 pandas\util\_decorators.py:14: in <module> from pandas._libs.properties import cache_readonly pandas\_libs\__init__.py:13: in <module> from pandas._libs.interval import Interval E ModuleNotFoundError: No module named 'pandas._libs.interval'

I have checked the module that was said to be missing and it exists. I searched and found this.

I have made the required change and will be pushing it to remote.

you need to follow the instructions here to run tests https://pandas.pydata.org/docs/dev/development/contributing_environment.html

MarcoGorelli · 2023-02-21T14:09:05Z

@MarcoGorelli Sorry, I have been away for a while. I have re-run and I still see the errors. Looking through to see where I might have gone wrong.

no worries :) I've suggested a couple of fixes, the rest looks good!

Changed dtype_mappings and CSS_EXPANSIONS respectively.

pandas/io/formats/css.py

Ran black pre-commit hook.

ch1booze · 2023-02-22T08:17:22Z

@MarcoGorelli All tests passed. I guess merging can now be done.

MarcoGorelli · 2023-02-22T09:21:00Z

yup, should be good, I'll just wait for a few more jobs to finish

MarcoGorelli

Thanks @victorian177 !

Ran pre-commit and fixed all errors

6d05c42

MarcoGorelli changed the title ~~Ran pre-commit and fixed all errors~~ STYLE ruff: enable PIE Feb 16, 2023

MarcoGorelli suggested changes Feb 16, 2023

View reviewed changes

Formatted using black

7bb6483

MarcoGorelli reviewed Feb 16, 2023

View reviewed changes

ch1booze added 2 commits February 16, 2023 18:25

Update .pre-commit-config.yaml

e24c898

Removed the fix arguments.

Update stata.py

73b3951

Merge branch 'main' into main

0970ed5

mroeschke added the Code Style Code style, linting, code_checks label Feb 16, 2023

ch1booze and others added 2 commits February 16, 2023 23:16

revert changes to doc/source/user_guide/style.ipynb

b4de25d

Merge branch 'main' into main

0bf01bb

MarcoGorelli approved these changes Feb 16, 2023

View reviewed changes

Merge branch 'main' into main

6942810

MarcoGorelli suggested changes Feb 17, 2023

View reviewed changes

ch1booze and others added 4 commits February 20, 2023 13:19

Merge branch 'main' into main

416f49f

Merge branch 'main' into main

f58be1c

Update test_indexing.py

da52ef7

Merge branch 'main' of https://github.com/victorian177/pandas

2d3ffb9

MarcoGorelli reviewed Feb 21, 2023

View reviewed changes

Update test_to_records.py and css.py

2cdc091

Changed dtype_mappings and CSS_EXPANSIONS respectively.

MarcoGorelli suggested changes Feb 21, 2023

View reviewed changes

pandas/io/formats/css.py Outdated Show resolved Hide resolved

Update css.py

6cd00ab

Ran black pre-commit hook.

Merge branch 'main' into main

df2d3fd

MarcoGorelli added this to the 2.1 milestone Feb 22, 2023

MarcoGorelli approved these changes Feb 22, 2023

View reviewed changes

MarcoGorelli merged commit 1b5370a into pandas-dev:main Feb 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STYLE ruff: enable PIE #51434

STYLE ruff: enable PIE #51434

ch1booze commented Feb 16, 2023 •

edited

Loading

MarcoGorelli left a comment

MarcoGorelli Feb 16, 2023

ch1booze Feb 16, 2023

MarcoGorelli Feb 16, 2023

ch1booze Feb 16, 2023

MarcoGorelli Feb 16, 2023

MarcoGorelli Feb 16, 2023

ch1booze Feb 16, 2023

MarcoGorelli Feb 16, 2023

ch1booze Feb 16, 2023

MarcoGorelli Feb 16, 2023

ch1booze Feb 16, 2023 •

edited

Loading

MarcoGorelli Feb 16, 2023

MarcoGorelli Feb 16, 2023

ch1booze commented Feb 16, 2023

MarcoGorelli commented Feb 16, 2023

MarcoGorelli commented Feb 16, 2023

ch1booze commented Feb 16, 2023

MarcoGorelli left a comment

ch1booze commented Feb 17, 2023

MarcoGorelli Feb 17, 2023

MarcoGorelli Feb 21, 2023

ch1booze Feb 21, 2023

MarcoGorelli commented Feb 17, 2023

ch1booze commented Feb 21, 2023

MarcoGorelli Feb 21, 2023

ch1booze Feb 21, 2023

MarcoGorelli Feb 21, 2023

MarcoGorelli commented Feb 21, 2023

ch1booze commented Feb 22, 2023

MarcoGorelli commented Feb 22, 2023

MarcoGorelli left a comment

		"column_dtypes": DictLike(**{"A": np.int8, "B": np.float32}),
		"column_dtypes": DictLike({"A": np.int8, "B": np.float32}),

STYLE ruff: enable PIE #51434

STYLE ruff: enable PIE #51434

Conversation

ch1booze commented Feb 16, 2023 • edited Loading

MarcoGorelli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ch1booze Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ch1booze commented Feb 16, 2023

MarcoGorelli commented Feb 16, 2023

MarcoGorelli commented Feb 16, 2023

ch1booze commented Feb 16, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

ch1booze commented Feb 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcoGorelli commented Feb 17, 2023

ch1booze commented Feb 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcoGorelli commented Feb 21, 2023

ch1booze commented Feb 22, 2023

MarcoGorelli commented Feb 22, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

ch1booze commented Feb 16, 2023 •

edited

Loading

ch1booze Feb 16, 2023 •

edited

Loading