-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: use _get_axis_number
instead of if axis in [...]
:
#43048
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
Comments
I think this applies to all code, not just the style, is that right? Or would you like to keep this issue focused on just the styler? |
@rhshadrach I don't know. I know that it applies to much of the stuff I wrote in |
yeah this was mainly for Styler, but its possible we do this elsewhere in the codebase so should clean those up (and ideally have a pre-commit rule but might be non-trivial) |
Can I work on this issues? |
I noticed it in the tests recently, doing some quick searches I'm seeing a few instances in core as well. |
hey, can i have this issue? |
Yes you can!
…On Wed, 18 Aug 2021, 11:32 am Adini Hemanjali, ***@***.***> wrote:
hey, can i have this issue?
I'm quite new to this so I might need some guidance
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#43048 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARSLAGXRKPDJRYQ2OK4A26TT5NEIRANCNFSM5CGBDK6Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
As has been commented here there are likely many places this pattern is present in the code and needs addressing. In order to find bits to work on you will have to search for a pattern in the code |
Will do, thanks!
…On Wed, 18 Aug 2021, 11:53 pm attack68, ***@***.***> wrote:
As has been commented here there are likely many places this pattern is
present in the code and needs addressing.
The best PRs will be to work on a single file and correct all the
offending occurrences within that file. There may be only one item to
address or several.
After the PR is approved and merged, we will note it down here as having
been addressed.
My example above shows how to do this for a file style.py.
In order to find bits to work on you will have to search for a pattern in
the code axis in and then see if you can correct it as per my example.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#43048 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARSLAGV2237W7SEON6SXRI3T5P3BNANCNFSM5CGBDK6Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
take |
@attack68 I have assigned myself to work on this issue, should I move forward with it? |
Yes, go ahead.
…On Thu, 19 Aug 2021, 5:41 pm Rohan Sirohia, ***@***.***> wrote:
@attack68 <https://github.com/attack68> I have assigned myself to work on
this issue, should I move forward with it?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#43048 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARSLAGVJSTRTVF5ZP2PLAFTT5TYF5ANCNFSM5CGBDK6Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
Anyone is welcome to work on this issue. This will not be solved by just one PR, there are likely many places and many files where the same work needs to be done. Therefore many people can work on this, simultaneously. |
Hello, could I help with this issue? |
Fixed the issue : #43220 |
Hey, i want to work towards resolving this issue. Please assign this to me |
take |
Is this instance has to be replaced in test files also or only in generic and style files? |
Ok, I'm pretty new to the whole scene, and directly replacing with get_axis_number was presenting with errors, so I tried importing the data frames directly, and that method did not give excess errors, so, I'm almost done |
I m first time to this contribution field, I need some guidance to work on this issue. can I? |
Please see the revised description of this issue. |
I am new to OpenSource Contributions . I want to work on this feature . |
take |
take |
take |
Did a quick search of the codebase and outside of all the test files this seems to have been addressed by all contributions now. |
Uh oh!
There was an error while loading. Please reload this page.
This PR regards searching the non-test codebase (i.e. don't find code examples in pandas/tests) for examples like the following:
Try to find and replace logic like:
with the standard codebase method
get_axis_number(axis)
. This is a DataFrame method so often you need use local DataFrame objects to call this method.It may also be necessary to change
axis
args in internal methods to consistently accept the same input format compatible with this method, i.e. axis as an integer (0 or 1).This is an open issue and contributions are welcome to solve individual files in the code. As an example see #43078 or other merged contributions below.
style.py
REF:axis in [...]
becomes_get_axis_number(axis)
in Styler #43078test_label_or_level_utils.py
REF: Replacingaxis in
with_get_axis_number(axis)
#43131axis in [...]
becomes_get_axis_number(axis)
in Styler #43121The text was updated successfully, but these errors were encountered: