Skip to content

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

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.

Sign up for GitHub

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 7 commits into from
Dec 19, 2022
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d7bab20
CLN: Fix pylint undefined-loop-variable warnings
Leviob Nov 17, 2022
8c91f39
Fix undefined-loop-variable pylint warning
Leviob Dec 1, 2022
fc6998e
Merge branch 'pandas-dev:main' into fix-undefined-loop-variable
Leviob Dec 1, 2022
4f0e90d
Merge branch 'fix-undefined-loop-variable' of github.com:Leviob/panda…
Leviob Dec 1, 2022
297889e
Merge branch 'fix-undefined-loop-variable' of github.com:Leviob/panda…
Leviob Dec 1, 2022
ed65d3d
Merge branch 'fix-undefined-loop-variable' of github.com:Leviob/panda…
Leviob Dec 15, 2022
10b612a
CLN: Format props f-strings for clarity
Leviob Dec 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pandas/core/util/hashing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def combine_hash_arrays(
out ^= a
out *= mult
mult += np.uint64(82520 + inverse_i + inverse_i)
assert i + 1 == num_items, "Fed in wrong num_items"
assert len(arrays) == num_items, "Fed in wrong num_items"
out += np.uint64(97531)
return out

Expand Down
2 changes: 1 addition & 1 deletion pandas/io/excel/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ def parse(
if ret_dict:
return output
else:
return output[asheetname]
return output[asheetname] # pylint: disable=undefined-loop-variable
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@uzzell uzzell Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.



@doc(storage_options=_shared_docs["storage_options"])
Expand Down
3 changes: 1 addition & 2 deletions pandas/io/formats/latex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,7 @@ def _get_strcols(self) -> list[list[str]]:
def pad_empties(x):
for pad in reversed(x):
if pad:
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:]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


gen = (pad_empties(i) for i in out)

Expand Down
4 changes: 2 additions & 2 deletions pandas/io/formats/printing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,9 @@ def best_len(values: list[str]) -> int:
for max_items in reversed(range(1, len(value) + 1)):
pprinted_seq = _pprint_seq(value, max_seq_items=max_items)
if len(pprinted_seq) < max_space:
head = [_pprint_seq(x, max_seq_items=max_items) for x in head]
tail = [_pprint_seq(x, max_seq_items=max_items) for x in tail]
break
head = [_pprint_seq(x, max_seq_items=max_items) for x in head]
tail = [_pprint_seq(x, max_seq_items=max_items) for x in tail]

summary = ""
line = space2
Expand Down
1 change: 1 addition & 0 deletions pandas/io/formats/style.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; "
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"z-index:2;"
),
Expand Down
3 changes: 3 additions & 0 deletions pandas/tests/frame/methods/test_to_dict_of_blocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def test_copy_blocks(self, float_frame):
_df.loc[:, column] = _df[column] + 1

# make sure we did not change the original DataFrame
# pylint: disable-next=undefined-loop-variable
assert not _df[column].equals(df[column])

def test_no_copy_blocks(self, float_frame, using_copy_on_write):
Expand All @@ -40,8 +41,10 @@ def test_no_copy_blocks(self, float_frame, using_copy_on_write):

if not using_copy_on_write:
# make sure we did change the original DataFrame
# pylint: disable-next=undefined-loop-variable
assert _df[column].equals(df[column])
else:
# pylint: disable-next=undefined-loop-variable
assert not _df[column].equals(df[column])


Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/frame/test_iteration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,4 @@ def test_sequence_like_with_categorical(self):
str(s)

for c, col in df.items():
str(s)
str(s) # pylint: disable=undefined-loop-variable
1 change: 1 addition & 0 deletions pandas/tests/io/formats/test_printing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand obj here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the help!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're welcome!


expected = {
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ disable = [
"subprocess-run-check",
"super-init-not-called",
"try-except-raise",
"undefined-loop-variable",
"unnecessary-lambda",
"unspecified-encoding",
"unused-argument",
Expand Down