Skip to content

REF get-axis-number #43263

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

Closed

Conversation

ForthenchoPacino
Copy link

@ForthenchoPacino ForthenchoPacino commented Aug 27, 2021

@pep8speaks
Copy link

pep8speaks commented Aug 27, 2021

Hello @ForthenchoPacino! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 317:1: W293 blank line contains whitespace

Line 1152:46: E225 missing whitespace around operator
Line 1154:46: E225 missing whitespace around operator

Line 998:57: E225 missing whitespace around operator
Line 1014:46: E225 missing whitespace around operator
Line 1025:46: E225 missing whitespace around operator
Line 1105:57: E225 missing whitespace around operator
Line 1118:64: E225 missing whitespace around operator
Line 1144:64: E225 missing whitespace around operator
Line 1169:64: E225 missing whitespace around operator

Line 50:57: E225 missing whitespace around operator
Line 53:42: E225 missing whitespace around operator

Line 105:46: E225 missing whitespace around operator
Line 107:46: E225 missing whitespace around operator

Line 167:46: E225 missing whitespace around operator
Line 172:46: E225 missing whitespace around operator
Line 186:46: E225 missing whitespace around operator
Line 193:46: E225 missing whitespace around operator

Line 37:46: E225 missing whitespace around operator

Line 550:46: E225 missing whitespace around operator

Line 491:42: E225 missing whitespace around operator

Comment last updated at 2021-08-28 04:40:43 UTC

@@ -23,6 +23,7 @@
)
import warnings
import weakref
from pandas import DataFrame
Copy link
Member

Choose a reason for hiding this comment

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

why?

elif axis in [1, "columns"]:
if DataFrame()._get_axis_number(axis)==0:
result = data.apply(func, axis=0, **kwargs)
if DataFrame()._get_axis_number(axis)==1:
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Contributor

Choose a reason for hiding this comment

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

don't make changes to style.py this already has PRs fixing this. please revert.

Copy link
Author

@ForthenchoPacino ForthenchoPacino Aug 30, 2021

Choose a reason for hiding this comment

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

aight. on it
just to confirm, though, were my changes correct? or were they unnecessary?

@jbrockmendel
Copy link
Member

What is the motivation for any of this?

@ForthenchoPacino
Copy link
Author

ForthenchoPacino commented Aug 28, 2021

Why, as in? Like, is it a really inefficient method, or was the whole thing not needed?

@@ -488,7 +488,7 @@ def test_rolling_axis_count(axis_frame):

axis = df._get_axis_number(axis_frame)

if axis in [0, "index"]:
if DataFrame()._get_axis_number(axis)==0:
Copy link
Contributor

Choose a reason for hiding this comment

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

axis has already been reduced to an integer in line 489. Sufficient for if axis == 0

Copy link
Author

Choose a reason for hiding this comment

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

Ah. So, has this specific edit been rendered unnecessary?

@@ -995,7 +995,7 @@ def test_consistency_for_boxed(box, int_frame_const_col):


def test_agg_transform(axis, float_frame):
other_axis = 1 if axis in {0, "index"} else 0
other_axis = 1 if DataFrame()._get_axis_number(axis)==0 else 0
Copy link
Contributor

Choose a reason for hiding this comment

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

the issue has been revised to avoid fixing test files. Too many new contributors were making mistakes by changing the nature of what was being tested.

Copy link
Author

Choose a reason for hiding this comment

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

So, no touching test files, then?

Copy link
Contributor

Choose a reason for hiding this comment

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

dont edit test files.

Copy link
Author

Choose a reason for hiding this comment

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

Aight. Won't. Thanks for telling. I'll revert the changes

@@ -547,7 +547,7 @@ def test_groupby_level_index_names(self, axis):
df = DataFrame({"exp": ["A"] * 3 + ["B"] * 3, "var1": range(6)}).set_index(
"exp"
)
if axis in (1, "columns"):
if DataFrame()._get_axis_number(axis)==0:
Copy link
Contributor

Choose a reason for hiding this comment

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

you can do this here, but use existing objects such as df._get_axis_number(axis) == 1. Also note you have got the 0 and 1 wrong here.

Copy link
Author

Choose a reason for hiding this comment

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

Ah. Thanks for point that out, I'll correct it and re-commit

@MarcoGorelli MarcoGorelli changed the title my fixes to the code REF get-axis-number Aug 30, 2021
@simonjayhawkins
Copy link
Member

Thanks @ForthenchoPacino for the PR. I think #43121 fixed the issue for pandas/io/formats/style.py and the other changes are to files in pandas/tests, see #43048 (comment). so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants