for issue #49656 STYLE enable pylint's redefined-outer-name #49708
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| from datetime import timedelta | ||
| from datetime import timedelta as td |
There was a problem hiding this comment.
would import datetime as dt and then dt.timedelta work?
pandas/core/generic.py
Outdated
Comment on lines
136
to
135
| common as com, | ||
| common, |
There was a problem hiding this comment.
does it work to just import the various functions from pandas.core.common? from pandas.core.common import get_rename_function, ...?
pandas/core/generic.py
Outdated
| com=com, | ||
| com=commmon, |
|
Done the changes sir |
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| indexing, | ||
| missing, | ||
| nanops, | ||
| sample, | ||
| ) | ||
| from pandas.core.common import get_rename_function as com |
There was a problem hiding this comment.
no just
from pandas.core.common import get_rename_function
and then use get_rename_function instead of com.get_rename_function
likewise for any other com. functions
pandas/core/generic.py
Outdated
| @@ -6318,7 +6317,8 @@ def _convert( | |||
| self: NDFrameT, | |||
| datetime: bool_t = False, | |||
| numeric: bool_t = False, | |||
| timedelta: bool_t = False, | |||
| # td: bool_t = False, | |||
pandas/core/generic.py
Outdated
| @@ -6318,7 +6317,8 @@ def _convert( | |||
| self: NDFrameT, | |||
| datetime: bool_t = False, | |||
| numeric: bool_t = False, | |||
| timedelta: bool_t = False, | |||
| # td: bool_t = False, | |||
| dt.timedelta: bool_t = False, | |||
There was a problem hiding this comment.
this shouldn't change, it's a function argument
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| @@ -2,9 +2,8 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| import collections | |||
| from datetime import timedelta | |||
| from datetime ad dt | |||
There was a problem hiding this comment.
this isn't valid syntax...
yes sir |
|
Getting closer - can you also run |
sir this error is coming |
MarcoGorelli
suggested changes
Nov 15, 2022
| @@ -2,9 +2,8 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| import collections | |||
| from datetime import timedelta | |||
| import datetime as dt | |||
There was a problem hiding this comment.
now you'll need to replace timedelta (not the function argument) with dt.timedelta
|
|
Looks like an issue with Anyway, for this issue, if you look at https://github.com/pandas-dev/pandas/actions/runs/3471395358/jobs/5800830394 it'll show you what the remaining issues are and what needs changing |
yes sir |
no such issure is reported there sir, should I write problem with autotyping and attach the output for reporting it. |
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| @@ -142,6 +143,14 @@ | |||
| from pandas.core.array_algos.replace import should_use_regex | |||
| from pandas.core.arrays import ExtensionArray | |||
| from pandas.core.base import PandasObject | |||
| from pandas.core.common import ( | |||
There was a problem hiding this comment.
I think we'll need to add # noqa: PDF018 as a comment here, we can't use the com alias
yes |
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| @@ -2,9 +2,8 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| import collections | |||
| from datetime import timedelta | |||
| import datetime as _dt | |||
There was a problem hiding this comment.
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| @@ -31,7 +30,10 @@ | |||
|
|
|||
| from pandas._config import config | |||
|
|
|||
| from pandas._libs import lib | |||
| from pandas._libs import ( | |||
| json, | |||
There was a problem hiding this comment.
why are you importing json from here instead of import json? it's different
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
Comment on lines
5687
to
5696
| rs = com.random_state(random_state) | ||
| rs = random_state(random_state) |
There was a problem hiding this comment.
now the random_state argument from this function shadows the random_state import
Might have to change the pandas.core.common import back to from pandas.core import common, but add the noqa command to that line to silence the flake8 warning of the missing as com alias
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| @@ -130,10 +130,10 @@ | |||
| notna, | |||
| ) | |||
|
|
|||
| from pandas.core import common # noqa: PDF018 | |||
There was a problem hiding this comment.
remove this, the next lines should be
from pandas.core import (
algorithms as algos,
arraylike,
common, # noqa: PDF018
pandas/core/generic.py
Outdated
Comment on lines
145
to
152
| from pandas.core.common import ( # noqa: PDF018 | ||
| apply_if_callable, | ||
| count_not_none, | ||
| get_rename_function, | ||
| index_labels_to_array, | ||
| maybe_make_list, | ||
| pipe, | ||
| ) |
pandas/core/generic.py
Outdated
| @@ -1009,7 +1017,7 @@ def _rename( | |||
| continue | |||
|
|
|||
| ax = self._get_axis(axis_no) | |||
| f = com.get_rename_function(replacements) | |||
| f = get_rename_function(replacements) | |||
There was a problem hiding this comment.
this will have to be common.get_rename_function (similarly for the others)
MarcoGorelli
suggested changes
Nov 15, 2022
pandas/core/generic.py
Outdated
| @@ -32,6 +31,7 @@ | |||
| from pandas._config import config | |||
|
|
|||
| from pandas._libs import lib | |||
| import json | |||
There was a problem hiding this comment.
the imports are in the wrong order, if you run
pre-commit run black --files pandas/core/generic.py
pre-commit run isort --files pandas/core/generic.py
pre-commit run flake8 --files pandas/core/generic.py
before committing it should fix up most of the reported issues
There was a problem hiding this comment.
Done sir, and ran these tests all of them passed
MarcoGorelli
approved these changes
Nov 15, 2022
There was a problem hiding this comment.
Thanks @ramvikrams for sticking with this, looks good now! 💪
|
All thanks to you sir for guiding me. |
MarcoGorelli
pushed a commit
to MarcoGorelli/pandas
that referenced
this pull request
Nov 18, 2022
…pandas-dev#49708) * t1 * update * update * Update generic.py * update * Update generic.py * Update generic.py * Update generic.py * Update generic.py * Update generic.py * Update generic.py * Update generic.py
mliu08
pushed a commit
to mliu08/pandas
that referenced
this pull request
Nov 27, 2022
…pandas-dev#49708) * t1 * update * update * Update generic.py * update * Update generic.py * Update generic.py * Update generic.py * Update generic.py * Update generic.py * Update generic.py * Update generic.py
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@MarcoGorelli Sir in the file
pandas\core\generic.pyI had one doubt for line 7684and I could not understand how todo the correct changes in
pandas\core\resmple.py