Skip to content
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

STYLE Increase coverage of inconsistent-namespace-usage #39992

Open
MarcoGorelli opened this issue Feb 23, 2021 · 7 comments
Open

STYLE Increase coverage of inconsistent-namespace-usage #39992

MarcoGorelli opened this issue Feb 23, 2021 · 7 comments
Milestone

Comments

@MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli commented Feb 23, 2021

If you already have experience contributing, then please leave this issue to newcomers - thanks 👍


In .pre-commit-config.yaml, the check inconsistent-namespace-usage is limited to pandas/tests/frame/. Let's expand it to the rest of pandas/tests/.

The task here is:

  1. make sure you're familiar with the contributing guide
  2. choose a folder from the list below (e.g. series) and add it to the check in .pre-commit-config.yaml. E.g., if you chose series, that would be:
    -   id: inconsistent-namespace-usage
        name: 'Check for inconsistent use of pandas namespace in tests'
        entry: python scripts/check_for_inconsistent_pandas_namespace.py
        language: python
        types: [python]
        files: ^pandas/tests/(frame|series)/
  1. temporarily add tokenize-rt as an additional dependency and --replace as an arg (these will be removed later):
    -   id: inconsistent-namespace-usage
        name: 'Check for inconsistent use of pandas namespace in tests'
        entry: python scripts/check_for_inconsistent_pandas_namespace.py
        language: python
        types: [python]
        files: ^pandas/tests/(frame|series)/
        additional_dependencies: [tokenize-rt]
        args: [--replace]
  1. run this check on all files with pre-commit, i.e.:
  $ pre-commit run inconsistent-namespace-usage --all-files
  [INFO] Initializing environment for local:tokenize-rt.
  [INFO] Installing environment for local.
  [INFO] Once installed this environment will be reused.
  [INFO] This may take a few minutes...
  Check for inconsistent use of pandas namespace in tests.......................Failed
  - hook id: inconsistent-namespace-usage
  - files were modified by this hook
  1. run it again - this time, it should pass
$ pre-commit run inconsistent-namespace-usage --all-files
Check for inconsistent use of pandas namespace in tests.......................Passed

Check that the other hooks still pass - if you enable pre-commit (pre-commit install), then this will happen automatically when you try to commit your changes (see step 6)

  1. revert the changes to .pre-commit-config.yaml (we'll update it once these changes are all so as to avoid a barrage of merge conflicts)
  2. commit your changes, and open a pull request. Check over your changes, make sure they look sensible, let me know if not

Here are the folders in pandas/tests which this check needs expanding to:

  • series
  • scalar
  • resample
  • util
  • tseries
  • dtypes
  • arrays
  • plotting
  • tools
  • indexing
  • strings
  • reductions
  • frame
  • arithmetic
  • config
  • api
  • generic
  • io
  • base
  • window
  • internals
  • reshape
  • computation
  • apply
  • groupby
  • tslibs
  • libs
  • indexes
  • extension

No need to ask for permission to work on this, just leave a comment letting people know which folder(s) from the list above you're working on

@MarcoGorelli MarcoGorelli added this to the 1.3 milestone Feb 23, 2021
@MarcoGorelli MarcoGorelli added the Style label Feb 23, 2021
@alexprincel
Copy link
Contributor

@alexprincel alexprincel commented Feb 23, 2021

I ran the script for "extension" and it made no change to files in that folder. I checked my procedure against your example above to make sure this was not the result of a bad manipulation. When changing to series I get the same output as in your post.

@alexprincel
Copy link
Contributor

@alexprincel alexprincel commented Feb 23, 2021

Currently working on indexes, will submit a PR as there were changes on that folder.

alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 23, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 23, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 23, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 23, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 23, 2021
punitvara added a commit to punitvara/pandas that referenced this issue Feb 24, 2021
Fix inconsistent namespace in apply directory.
punitvara added a commit to punitvara/pandas that referenced this issue Feb 24, 2021
Fix inconsistent namespace in apply directory.
punitvara added a commit to punitvara/pandas that referenced this issue Feb 24, 2021
Fix inconsistent namespace in apply directory.
MarcoGorelli pushed a commit that referenced this issue Feb 24, 2021
Fix inconsistent namespace in apply directory.
MarcoGorelli pushed a commit that referenced this issue Feb 24, 2021
Fix inconsistent namespace in apply directory.
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 25, 2021
@jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Feb 25, 2021

@MarcoGorelli the automatic script only changes it from pd.DataFrame(..) to DataFrame(..) and not the other way around? (as previously, we only agreed to be consistent within a file, not necessarily to change all files to use the non-pd. way)

@MarcoGorelli
Copy link
Member Author

@MarcoGorelli MarcoGorelli commented Feb 25, 2021

@jorisvandenbossche if there's both, it'll choose the non-pd. one. If the file is already consistently using the pd. one, then it'll leave it untouched.

e.g.

import pandas as pd

def test_0():
    pd.DataFrame()

def test_1():
    pd.DataFrame()

would be left untouched (i.e. it would keep being consistent within the file), but

import pandas as pd
from pandas import DataFrame

def test_0():
    pd.DataFrame()

def test_1():
    DataFrame()

would become

import pandas as pd
from pandas import DataFrame

def test_0():
    DataFrame()

def test_1():
    DataFrame()
@alexprincel
Copy link
Contributor

@alexprincel alexprincel commented Feb 25, 2021

libs and tslibs passed without fixes,

alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 25, 2021
@alexprincel
Copy link
Contributor

@alexprincel alexprincel commented Feb 25, 2021

computation passed without fixes

MarcoGorelli pushed a commit that referenced this issue Feb 25, 2021
MarcoGorelli pushed a commit that referenced this issue Feb 25, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 25, 2021
@AA-Turner AA-Turner mentioned this issue Feb 25, 2021
1 of 2 tasks complete
MarcoGorelli added a commit that referenced this issue Feb 28, 2021
Co-authored-by: Marco Gorelli <marcogorelli@protonmail.com>
@na2shell na2shell mentioned this issue Mar 2, 2021
1 of 4 tasks complete
parkerashlan added a commit to parkerashlan/pandas that referenced this issue Mar 2, 2021
MarcoGorelli pushed a commit that referenced this issue Mar 2, 2021
@Varun270
Copy link

@Varun270 Varun270 commented Mar 2, 2021

can someone please tell me whether this issue is fixed or not .

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

Successfully merging a pull request may close this issue.

None yet
4 participants