CLN: Fix pylint undefined-loop-variable warnings #49740

Leviob · 2022-11-17T03:19:47Z

Contributes to #48855. I added in-line pylint disables or refactored code where relevant.

I'm still a little unsure about the some of these warnings, any help is appreciated.

pandas/core/util/hashing.py

pandas/tests/frame/test_iteration.py

Leviob · 2022-11-17T03:27:59Z

pandas/io/formats/style.py

@@ -2298,6 +2298,7 @@ def set_sticky(
                            "selector": f"thead tr:nth-child({obj.nlevels+1}) th",
                            "props": props
                            + (
+                                # pylint: disable=undefined-loop-variable
                                f"top:{(i+1) * pixel_size}px; height:{pixel_size}px; "


I don't understand what's going on in this file. The i causing the warning is defined on line 2284.

it's just that it's outside of the loop

Leviob · 2022-11-17T03:31:04Z

pandas/io/excel/_base.py

@@ -883,7 +883,7 @@ def parse(
        if ret_dict:
            return output
        else:
-            return output[asheetname]
+            return output[asheetname]  # pylint: disable=undefined-loop-variable


I'm not sure about this one. asheetname is defined on line 731.

I think that the key here is the sequence of conditional statements that start in line 715. In the first two cases, ret_dict is set to True, which means that the method won't reach this branch. In the last two cases, we have sheets = [sheet_name]. Because sheets is non-empty, asheetname will be defined.

Makes sense, thanks.

is there a way to refactor such as to not have to turn off the warning? the warning is there because asheetname is being used outside the scope of the loop

Aside from being a silly example, is this an undesired pattern?

for x in ['a', 'b', 'c']: if random.choice([True, False]): break return x

Here the loop variable is guaranteed to exist after the for-loop (the same as in this code here).

I think it's just to help prevent Edge cases like

for x in mylist: print(x) return x

in which x in the last line would be undefined if mylist was empty

Then I think it's okay to disable the warning in this case (the for loop is guaranteed to be entered). I'm also very much in favor of refactoring this code, but for reasons outside of the scope of this issue.

pandas/tests/frame/methods/test_to_dict_of_blocks.py

Leviob · 2022-11-17T03:35:37Z

pandas/tests/io/formats/test_printing.py

@@ -141,6 +141,7 @@ def test_publishes(self, ip):
        with_latex = pd.option_context("display.latex.repr", True)

        with opt, with_latex:
+            # pylint: disable-next=undefined-loop-variable
            formatted = ipython.display_formatter.format(obj)


I don't understand obj here.

obj is an element of objects, which is defined as objects = [df["A"], df, df]. So, this one is a false positive.

Thanks for the help!

You're welcome!

…s-leviob into fix-undefined-loop-variable

MarcoGorelli · 2022-12-12T19:52:02Z

pandas/io/excel/_base.py

@@ -883,7 +883,7 @@ def parse(
        if ret_dict:
            return output
        else:
-            return output[asheetname]
+            return output[asheetname]  # pylint: disable=undefined-loop-variable


is there a way to refactor such as to not have to turn off the warning? the warning is there because asheetname is being used outside the scope of the loop

MarcoGorelli

Thanks for working on this

This causes quite a few false-positives - shall we keep it turned off, but keep the two refactors you found (which are quite nice)?

MarcoGorelli · 2022-12-13T09:48:11Z

pandas/io/formats/latex.py

-                        break
-                return [x[0]] + [i if i else " " * len(pad) for i in x[1:]]
+                        return [x[0]] + [i if i else " " * len(pad) for i in x[1:]]


I think that's reasonable. But I also want to change the variable s in this test to col. I think the current variable probably a mistake that this warning brought to my attention.

pandas/pandas/tests/frame/test_iteration.py

Line 162 in d7bab20

str(s) # pylint: disable=undefined-loop-variable

yeah sounds good - looks like the test is just there to check that iteration works anyway

…s-leviob into fix-undefined-loop-variable

Leviob · 2022-12-15T20:19:25Z

pandas/io/formats/style.py

                            + (
-                                f"top:{(i+1) * pixel_size}px; height:{pixel_size}px; "
+                                f"top:{(len(levels_)) * pixel_size}px; \
+                                    height:{pixel_size}px; "


My reasoning here is that the for loop will have already completed by this point. If levels_ were an empty list, i would be undefined, whereas len(levels_) would be 0.

can you run the other pre-commit checks please? pretty sure this'll be reformatted

I ran pre-commit run pylint --hook-stage manual --files pandas/io/formats/style.py and pre-commit run --all-files and they all passed. What other checks need to be run?

ah my bad, I thought black forbade this \, but it looks like it's fine if it's in a string

this is probably fine, I'll give it another look tomorrow

Without running this, it's not clear to me that it's the same

Could you write it as something like

f"top:{(len(levels_)) * pixel_size}px; " f"height:{pixel_size}px; "

please?

The rest looks fine

Yep. Thanks for the help!

MarcoGorelli

thanks @Leviob

CLN: Fix pylint undefined-loop-variable warnings

d7bab20

Leviob commented Nov 17, 2022

View reviewed changes

pandas/core/util/hashing.py Outdated Show resolved Hide resolved

Leviob commented Nov 17, 2022

View reviewed changes

pandas/tests/frame/test_iteration.py Outdated Show resolved Hide resolved

Leviob commented Nov 17, 2022

View reviewed changes

pandas/tests/frame/methods/test_to_dict_of_blocks.py Outdated Show resolved Hide resolved

Leviob commented Nov 17, 2022

View reviewed changes

mroeschke added the Code Style Code style, linting, code_checks label Nov 17, 2022

Leviob and others added 3 commits November 30, 2022 16:34

Fix undefined-loop-variable pylint warning

8c91f39

Merge branch 'pandas-dev:main' into fix-undefined-loop-variable

fc6998e

Merge branch 'fix-undefined-loop-variable' of github.com:Leviob/panda…

4f0e90d

…s-leviob into fix-undefined-loop-variable

Leviob marked this pull request as ready for review December 1, 2022 00:38

MarcoGorelli self-requested a review December 1, 2022 09:25

MarcoGorelli requested changes Dec 12, 2022

View reviewed changes

MarcoGorelli requested changes Dec 13, 2022

View reviewed changes

Leviob added 2 commits December 15, 2022 12:11

Merge branch 'fix-undefined-loop-variable' of github.com:Leviob/panda…

297889e

…s-leviob into fix-undefined-loop-variable

Merge branch 'fix-undefined-loop-variable' of github.com:Leviob/panda…

ed65d3d

…s-leviob into fix-undefined-loop-variable

Leviob commented Dec 15, 2022

View reviewed changes

Leviob requested a review from MarcoGorelli December 15, 2022 20:21

MarcoGorelli requested review from MarcoGorelli and removed request for MarcoGorelli December 15, 2022 21:55

CLN: Format props f-strings for clarity

10b612a

MarcoGorelli added this to the 2.0 milestone Dec 19, 2022

MarcoGorelli approved these changes Dec 19, 2022

View reviewed changes

MarcoGorelli merged commit 3c55eee into pandas-dev:main Dec 19, 2022

uzzell mentioned this pull request Jan 24, 2023

Enable pylint undefined-loop-variable warning #50961

Merged

1 task

asishm-wk mentioned this pull request Apr 27, 2023

BUG: AttributeError: 'tuple' object has no attribute 'rstrip' in format_object_summary #52960

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: Fix pylint undefined-loop-variable warnings #49740

CLN: Fix pylint undefined-loop-variable warnings #49740

Leviob commented Nov 17, 2022 •

edited

Loading

Leviob Nov 17, 2022

MarcoGorelli Dec 13, 2022

Leviob Nov 17, 2022

uzzell Nov 28, 2022 •

edited

Loading

Leviob Dec 1, 2022

MarcoGorelli Dec 12, 2022

rhshadrach Dec 12, 2022

MarcoGorelli Dec 12, 2022

rhshadrach Dec 13, 2022

Leviob Nov 17, 2022

uzzell Nov 28, 2022

Leviob Dec 1, 2022

uzzell Dec 1, 2022

MarcoGorelli Dec 12, 2022

MarcoGorelli left a comment

MarcoGorelli Dec 13, 2022

Leviob Dec 13, 2022

MarcoGorelli Dec 13, 2022

Leviob Dec 15, 2022

MarcoGorelli Dec 15, 2022

Leviob Dec 15, 2022

MarcoGorelli Dec 15, 2022

MarcoGorelli Dec 15, 2022

MarcoGorelli Dec 18, 2022

Leviob Dec 18, 2022

MarcoGorelli left a comment

CLN: Fix pylint undefined-loop-variable warnings #49740

CLN: Fix pylint undefined-loop-variable warnings #49740

Conversation

Leviob commented Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uzzell Nov 28, 2022 • edited Loading

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

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

MarcoGorelli left a comment

Choose a reason for hiding this comment

Leviob commented Nov 17, 2022 •

edited

Loading

uzzell Nov 28, 2022 •

edited

Loading