STYLE #49656: fixed redefined-outer-name linting issue to format.py #49937
Conversation
…class __call__ function
…d outer name conflict with is_dates_only function
…ange in datetimes.py
…er name conflict with justify functions defined for TextAdjustment and EastAsianTextAdjustment classes
MarcoGorelli left a comment
MarcoGorelli left a comment There was a problem hiding this comment.
Thanks @MarieKMiko for working on this! Could you also do
I've left a couple of comments - could you also remove file names you've fixed from
pandas/.pre-commit-config.yaml
Lines 82 to 84 in 094b2c0
please?
pandas/io/formats/format.py Outdated
| | ||
| def get_format_datetime64( | ||
| is_dates_only: bool, nat_rep: str = "NaT", date_format: str | None = None | ||
| is_dates_only_result: bool, nat_rep: str = "NaT", date_format: str | None = None |
There was a problem hiding this comment.
ideally, let's not rename function arguments
There was a problem hiding this comment.
Agreed this is not ideal- not sure how to account for the issue that is_dates_only is both a function name and an argument in a different function, which seems to be what's causing the issue. Would it be better to rename the function rather than then argument?
There was a problem hiding this comment.
ah, I see what you mean, thanks. gosh, that's dirty 😄 taking a look
There was a problem hiding this comment.
The function is_dates_only is only used within this file - how about we just rename it to _is_dates_only, and then this argument can stay as it is?
There was a problem hiding this comment.
I think that could work - let me double check because there is another function somewhere called _is_dates_only but it may be in a different file
There was a problem hiding this comment.
The function is_dates_only is only used within this file
sorry I was wrong, it's used inside that other function in fact
There was a problem hiding this comment.
In
pandas/pandas/io/formats/style_render.py
Line 1806 in 694c0cd
the argument slice is called slice_ to avoid shadowing the builtin slice - shall we do the same here and rename to is_dates_only_?
If you then remove format.py from
pandas/.pre-commit-config.yaml
Line 83 in 61e0db2
file, the rest looks good 👍
EDIT: in fact, looks like |^pandas/core/tools/datetimes\.py can also already be removed from the pre-commit-config file
pandas/io/formats/format.py Outdated
| | ||
| def get_format_datetime64( | ||
| is_dates_only: bool, nat_rep: str = "NaT", date_format: str | None = None | ||
| is_dates_only_result: bool, nat_rep: str = "NaT", date_format: str | None = None |
There was a problem hiding this comment.
In
pandas/pandas/io/formats/style_render.py
Line 1806 in 694c0cd
the argument slice is called slice_ to avoid shadowing the builtin slice - shall we do the same here and rename to is_dates_only_?
If you then remove format.py from
pandas/.pre-commit-config.yaml
Line 83 in 61e0db2
file, the rest looks good 👍
EDIT: in fact, looks like |^pandas/core/tools/datetimes\.py can also already be removed from the pre-commit-config file
…_datetime64 to is_dates_only
pandas/io/formats/format.py Outdated
| | ||
| | ||
| def is_dates_only(values: np.ndarray | DatetimeArray | Index | DatetimeIndex) -> bool: | ||
| def is_dates_only_(values: np.ndarray | DatetimeArray | Index | DatetimeIndex) -> bool: |
There was a problem hiding this comment.
ah sorry, I meant to rename the argument get_format_datetime64 (else this function will need renaming in the other file where it's used, i.e.
pandas/pandas/core/indexes/datetimes.py
Line 359 in 14dc069
so, this function would stay as is_dates_only, and
pandas/pandas/io/formats/format.py
Lines 1796 to 1798 in 1c51e60
would change to
def get_format_datetime64( is_dates_only_: bool, nat_rep: str = "NaT", date_format: str | None = None ) -> Callable: There was a problem hiding this comment.
Oh- got it- let me just swap that around
MarcoGorelli left a comment
MarcoGorelli left a comment There was a problem hiding this comment.
Nice! This is probably fine, if you address #49937 (review) as well then this is probably good to go
removed format and datetimes files
MarcoGorelli left a comment
MarcoGorelli left a comment There was a problem hiding this comment.
Looks good to me pending green, well done!
| Thanks @MarieKMiko |
3 participants
Fixed redefined-outer-name issue in:
redefined-outer-name t issues raised for:
justify function (import from pandas.io.formats.printing)
decimal (changed to from decimal import Decimal)
is_dates_only parameter conflict with function name
updated get_format_datetime64 function with renamed parameter
passed related tests in pytest pandas
running: pylint --disable=all --enable=redefined-outer-name pandas/io/
-- > Results in 10.00/10 rating