Skip to content

STYLE enable pylint's redefined-outer-name #49656

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
MarcoGorelli opened this issue Nov 11, 2022 · 45 comments · Fixed by #49960 or #50561
Closed

STYLE enable pylint's redefined-outer-name #49656

MarcoGorelli opened this issue Nov 11, 2022 · 45 comments · Fixed by #49960 or #50561
Labels
Code Style Code style, linting, code_checks good first issue

Comments

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Nov 11, 2022

This warning could potentially prevent bugs, so I'm pretty keen on putting in the effort to enable it.

Many people can work on this one together. To work on it, please:

  1. choose 2-3 files
  2. make sure you have pylint version 2.15.5 installed (pip install pylint==2.15.5) - check this is the same as the number in the .pre-commit-config.yaml file
  3. for each file, run pylint --disable=all --enable=redefined-outer-name <file>
  4. fix up the file until it passes
  5. stage, commit, push, open pull request

For example, suppose you choose the file pandas/io/json/_table_schema.py. Running pylint on it, you get:

$ pylint --disable=all --enable=redefined-outer-name pandas/io/json/_table_schema.py
************* Module pandas.io.json._table_schema
pandas/io/json/_table_schema.py:314:23: W0621: Redefining name 'json' from outer scope (line 15) (redefined-outer-name)

-----------------------------------
Your code has been rated at 9.92/10

If we look at that file, we see that there's a function

def parse_table_schema(json, precise_float):

which takes json as an argument, but the file also has

from pandas._libs import json

at the top.

The simplest fix is probably to just modify the import to be

from pandas._libs.json import loads

and then to remove

loads = json.loads

as loads is the only function used from pandas._libs.json.


Other notes:

  1. multiple people can work on this issue at the same time, please don't comment 'take'
  2. please choose no more than 3 files to work on in the same PR. Comment which files you'd like to take so we don't duplicate work

It's probably worth running pylint on a directory first to see what files need working on, e.g.

pylint --disable=all --enable=redefined-outer-name pandas/io/

Files that still need fixing:

  • pandas/_version.py
  • pandas/conftest.py
  • pandas/core/generic.py
  • pandas/core/internals/concat.py
  • pandas/core/reshape/merge.py
  • pandas/core/tools/datetimes.py
  • pandas/io/formats/format.py
  • pandas/io/formats/style.py
  • pandas/io/json/_json.py
  • pandas/io/xml.py
  • pandas/util/_decorators.py
  • pandas/util/_doctools.py

Let's ignore test files for now

@grtcoder
Copy link
Contributor

Hey, I would like to work on this

@MarcoGorelli
Copy link
Member Author

go ahead - as mentioned in the issue:

please choose no more than 3 files to work on in the same PR. Comment which files you'd like to take so we don't duplicate work

@ramvikrams
Copy link
Contributor

ramvikrams commented Nov 12, 2022

I'll take pandas\core\generic.py

@grtcoder
Copy link
Contributor

grtcoder commented Nov 12, 2022

I'll take

  1. pandas/core/arrays/datetimelike.py
  2. pandas/core/base.py
  3. pandas/core/computation/pytables.py

grtcoder added a commit to grtcoder/pandas that referenced this issue Nov 12, 2022
Fixed warnings for the following files :-
- pandas/core/arrays/datetimelike.py
- pandas/core/base.py
- pandas/core/computation/pytables.py
grtcoder added a commit to grtcoder/pandas that referenced this issue Nov 12, 2022
Fixed warnings for the following files :-
- pandas/core/arrays/datetimelike.py
- pandas/core/base.py
- pandas/core/computation/pytables.py
@Dhrubajyoti-Giri
Copy link

I'll take pandas/core/internals/concat.py, pandas/core/reshape/merge.py, pandas/io/json/_json.py.

@bhaveshrp
Copy link
Contributor

bhaveshrp commented Nov 12, 2022

I'll take

  1. pandas\core\frame.py
  2. pandas\core\dtypes\inference.py

@alphacrack
Copy link
Contributor

alphacrack commented Nov 12, 2022

I'll take:

  1. pandas/core/dtypes/cast.py
  2. pandas/core/dtypes/dtypes.py
  3. pandas/core/indexes/base.py

@aditya-anulekh
Copy link
Contributor

Added a pull request fixing the following files

  • pandas/core/indexes/datetimes.py
  • pandas/io/formats/xml.py
  • pandas/io/json/_table_schema.py

grtcoder added a commit to grtcoder/pandas that referenced this issue Nov 13, 2022
Fixed warnings for the following files :-
- pandas/core/arrays/datetimelike.py
- pandas/core/base.py
- pandas/core/computation/pytables.py
grtcoder added a commit to grtcoder/pandas that referenced this issue Nov 13, 2022
Fixed warnings for the following files :-
- pandas/core/arrays/datetimelike.py
- pandas/core/base.py
- pandas/core/computation/pytables.py
grtcoder added a commit to grtcoder/pandas that referenced this issue Nov 13, 2022
Fixed warnings for the following files :-
- pandas/core/arrays/datetimelike.py
- pandas/core/base.py
- pandas/core/computation/pytables.py
grtcoder added a commit to grtcoder/pandas that referenced this issue Nov 13, 2022
Fixed warnings for the following files :-
 - pandas/core/arrays/datetimelike.py
 - pandas/core/base.py
 - pandas/core/computation/pytables.py
Iteration :- 2
MarcoGorelli pushed a commit that referenced this issue Nov 13, 2022
* STYLE: fix pylint redefined-outer-name warnings (#49656)

Fixed warnings for the following files :-
- pandas/core/arrays/datetimelike.py
- pandas/core/base.py
- pandas/core/computation/pytables.py

* STYLE: fix pylint redefined-outer-name warnings (#49656)

Fixed warnings for the following files :-
 - pandas/core/arrays/datetimelike.py
 - pandas/core/base.py
 - pandas/core/computation/pytables.py
Iteration :- 2
@nandinimalik16
Copy link

nandinimalik16 commented Nov 13, 2022

I will take

  1. pandas/_version.py
  2. pandas/conftest.py
  3. pandas/io/formats/format.py

@isaac-chung
Copy link
Contributor

isaac-chung commented Nov 13, 2022

I'll take:

  1. pandas/plotting/_matplotlib/converter.py
  2. pandas/plotting/_matplotlib/core.py
  3. pandas/plotting/_matplotlib/tools.py

@seanjedi
Copy link
Contributor

Hi, I'd like to work on:

  1. pandas/io/formats/style.py
  2. pandas/io/json/_json.py
  3. pandas/io/xml.py

@zemnly
Copy link
Contributor

zemnly commented Nov 15, 2022

Are there any files which haven't been taken so I can work on them?

@MarcoGorelli
Copy link
Member Author

Yeah there's still some, I've updated the issue with an up-to-date list

Note that https://github.com/pandas-dev/pandas/pull/49668/files is already tackling some, but the rest should be up for grabs

Thanks all for your help with this one 🙏

@zemnly
Copy link
Contributor

zemnly commented Nov 15, 2022

Thanks, I'd like to take :

  1. pandas/core/algorithms.py
  2. pandas/core/generic.py
  3. pandas/core/tools/datetimes.py

@calhockemeyer
Copy link
Contributor

I will take: pandas/core/resample.py. I think that's the only file that hasn't been taken

@bang128
Copy link
Contributor

bang128 commented Nov 15, 2022

Can I also contribute to pandas/core/algorithms.py?

@ramvikrams
Copy link
Contributor

Can I also contribute to pandas/core/algorithms.py?

Yes you can i'll leave that file

@bang128
Copy link
Contributor

bang128 commented Nov 15, 2022

Can I also contribute to pandas/core/algorithms.py?

Yes you can i'll leave that file

Thank you so much!

MarcoGorelli pushed a commit that referenced this issue Nov 23, 2022
* Fix concat.py

* #49656: update concat.py, fix merge.py

* #49656: update concat.py and merge.py
@MarcoGorelli
Copy link
Member Author

MarcoGorelli commented Nov 24, 2022

We're almost done, there's just:

  • pandas/io/formats/format.py
  • pandas/core/generic.py

left

If anyone wants to take them, and then (in the same PR) remove these lines from .pre-commit-config.yaml

|^pandas/core/tools/datetimes\.py
|^pandas/io/formats/format\.py
|^pandas/core/generic\.py

, then we can close the issue

@Thextan
Copy link
Contributor

Thextan commented Nov 24, 2022

I will make changes to pandas/core/generic.py and pandas/.pre-commit-config.yaml.
This is my first time contributing and I am not very familiar with git so, please help me out if I am doing anything wrong.

@angularOcean
Copy link
Contributor

@Thextan I think we need to fix both format.py and generic.py before we can remove the lines from .pre-commit-config.yaml

@Thextan
Copy link
Contributor

Thextan commented Nov 24, 2022

@angularOcean when I ran my check it looked like someone had already fixed io/formats/format.py. I could be mistaken about that. And my changes are not passing the checks, so I may not even be ready for this simple beginner task.

Also, I didn't think the changes to .pre-commit-config.yaml was related to the refactoring for redefine-outer-name.

@MarcoGorelli
Copy link
Member Author

I'll take a look tomorrow, but if you search for the pandas contributing guide you'll find instructions for how to run the precommit checks

@Thextan
Copy link
Contributor

Thextan commented Nov 26, 2022

Please feel free to pick up the changes to core/generic.py, io/formats/format.py (if it still needs work), and .pre-commit-config.yaml. I am reading through the contributing guide to make sure I am doing things correctly.

mliu08 pushed a commit to mliu08/pandas that referenced this issue Nov 27, 2022
…andas-dev#49662)

* STYLE: fix pylint redefined-outer-name warnings (pandas-dev#49656)

Fixed warnings for the following files :-
- pandas/core/arrays/datetimelike.py
- pandas/core/base.py
- pandas/core/computation/pytables.py

* STYLE: fix pylint redefined-outer-name warnings (pandas-dev#49656)

Fixed warnings for the following files :-
 - pandas/core/arrays/datetimelike.py
 - pandas/core/base.py
 - pandas/core/computation/pytables.py
Iteration :- 2
mliu08 pushed a commit to mliu08/pandas that referenced this issue 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
mliu08 pushed a commit to mliu08/pandas that referenced this issue Nov 27, 2022
mliu08 pushed a commit to mliu08/pandas that referenced this issue Nov 27, 2022
* Fix concat.py

* pandas-dev#49656: update concat.py, fix merge.py

* pandas-dev#49656: update concat.py and merge.py
@MarcoGorelli
Copy link
Member Author

@Thextan not sure what you mean, sorry - your PR has been reviewed, do you plan to address the review? If not, no worries, that's fine, but please close the PR so others can work on it

@MarieKMiko
Copy link
Contributor

If there are still changes to be made to pandas/io/formats/format.py or pandas/core/generic.py, I'm happy to take these on

@MarcoGorelli
Copy link
Member Author

Yup, they're still open, if you wanted to submit a PR that'd be great, feel free to reach out (here or on the slack) if anything trips you up

@antocommi
Copy link

i'll take pandas/core/generic.py

@bang128
Copy link
Contributor

bang128 commented Nov 28, 2022

Have anyone taken pandas/io/formats/format.py yet? If not, can I take it?

This was referenced Nov 29, 2022
mroeschke pushed a commit that referenced this issue Nov 29, 2022
…49937)

* changed decimal module import, Decimal function call in EngFormatter class __call__ function

* changed name of parameter for get_format_datetime64 to avoid redefined outer name conflict with is_dates_only function

* change in argument name for call to get_format_datetime64 to match change in datetimes.py

* removed import - justify function not used and creating redefined outer name conflict with justify functions defined for TextAdjustment and EastAsianTextAdjustment classes

* replaced justify import with alias to avoid redefined-outer-name conflict

* renamed imports from printing module

* function renamed to is_dates_only_, returned argument for get_formate_datetime64 to is_dates_only

* changes in datetimes.py to reflect function and argument names in format.py

* argument and function names swapped

* function name is is_dates_only, argument for get_format_datetime64 is is_dates_only_

* Update .pre-commit-config.yaml

removed format and datetimes files
@MarcoGorelli
Copy link
Member Author

MarcoGorelli commented Nov 29, 2022

only pandas/core/generic.py is left, if anyone wants to finish off this issue - doesn't look like there's any PR open

@bang128
Copy link
Contributor

bang128 commented Nov 29, 2022

I can take it if no one is working on it

@MarcoGorelli
Copy link
Member Author

go ahead thanks

bang128 added a commit to bang128/pandas that referenced this issue Nov 29, 2022
MarcoGorelli pushed a commit that referenced this issue Nov 29, 2022
* Update format.py

* Update format.py 9.99/10

* Update generic.py

* Return format.py

* Update generic.py

* Update generic.py based on request change

* Done pre-commit
@MarcoGorelli
Copy link
Member Author

Looks like we forgot to remove this line

|^pandas/core/generic\.py

anyone feel like opening a PR to remove it?

@ramvikrams
Copy link
Contributor

pandas/.pre-commit-config.yaml

Done

MarcoGorelli pushed a commit that referenced this issue Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks good first issue
Projects
None yet