Skip to content

STYLE place standard library imports at top of file #49647

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 · 13 comments · Fixed by #50116
Closed

STYLE place standard library imports at top of file #49647

MarcoGorelli opened this issue Nov 11, 2022 · 13 comments · Fixed by #50116
Labels
Code Style Code style, linting, code_checks good first issue

Comments

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Nov 11, 2022

Imports should typically be placed at the top of files. Sometimes, imports are placed inside functions to:

  • avoid circular imports
  • avoid ImportError if it's an optional dependency

Standard library imports should really always be at the top of files.

Noticed in #49645 that this is often not the case

I've made a script to automate detecting when this is the case. So the task is:

git checkout -b standard-library-imports main
git pull [email protected]:MarcoGorelli/pandas.git standard-library-imports
git reset --hard FETCH_HEAD
pre-commit run stdlib-imports --all-files

Then, fixup any errors that are reported. Finally, stage your changes, commit them, push them to your fork, and open a pull request

Feel free to reach out if you into any issues along the way

If any wants to take this, it would be a nice and welcome clean up!


EDIT: after going through a PR, I'm not sure it's worth introducing a check for this - but we can still take some of the cleanups it found

@MarcoGorelli MarcoGorelli added Code Style Code style, linting, code_checks good first issue labels Nov 11, 2022
@mroeschke
Copy link
Member

Noting in the past that some stdlib imports were deferred (i.e. not at the top level) to help import time (not sure if this is still the case)

pandas/pandas/io/common.py

Lines 259 to 266 in e41b6d7

def urlopen(*args, **kwargs):
"""
Lazy-import wrapper for stdlib urlopen, as that imports a big chunk of
the stdlib.
"""
import urllib.request
return urllib.request.urlopen(*args, **kwargs)

@MarcoGorelli
Copy link
Member Author

MarcoGorelli commented Nov 11, 2022

Ah thanks! Can probably just respect the blocklist

pandas/ci/code_checks.sh

Lines 48 to 50 in bcb8346

blocklist = {'bs4', 'gcsfs', 'html5lib', 'http', 'ipython', 'jinja2', 'hypothesis',
'lxml', 'matplotlib', 'openpyxl', 'py', 'pytest', 's3fs', 'scipy',
'tables', 'urllib.request', 'xlrd', 'xlsxwriter'}

EDIT: done

@grtcoder
Copy link
Contributor

Hey, I would like to work on this.

@MarcoGorelli
Copy link
Member Author

sure go ahead

@zemnly
Copy link
Contributor

zemnly commented Nov 14, 2022

Hi, can I take this ?

@MarcoGorelli
Copy link
Member Author

looks like @grtcoder is already working on it

@natmokval
Copy link
Contributor

Hi @grtcoder. Do you mind if I will work on this issue as well?

@MarcoGorelli
Copy link
Member Author

MarcoGorelli commented Nov 29, 2022

feel free to go ahead, doesn't look like it's being worked on

@Thextan
Copy link
Contributor

Thextan commented Nov 30, 2022

@natmokval I am starting on this one. I will start with the files under pandas/core/ and pandas/io

@Thextan
Copy link
Contributor

Thextan commented Nov 30, 2022

Finished all files except pandas/tests/*. I will work on those next.

@seanjedi
Copy link
Contributor

seanjedi commented Dec 1, 2022

Is there anything for this issue that I can work on?

@Thextan
Copy link
Contributor

Thextan commented Dec 1, 2022 via email

@seanjedi
Copy link
Contributor

seanjedi commented Dec 1, 2022

Alright, thanks for letting me know!

Thextan added a commit to Thextan/pandas that referenced this issue Dec 7, 2022
part 2 of 2 pandas-dev#49647\n\nClean up of standard library imports moving them to the top of the file
Thextan added a commit to Thextan/pandas that referenced this issue Dec 7, 2022
part 2 of 2 pandas-dev#49647\n\nClean up of standard library imports moving them to the top of the file
Thextan added a commit to Thextan/pandas that referenced this issue Dec 27, 2022
pandas-dev#49647\n\nMerge .pre-commit-config.yaml with current version.
Thextan added a commit to Thextan/pandas that referenced this issue Dec 27, 2022
pandas-dev#49647\n\nMerge pandas/tests/series/test_arithetic.py  with current version.
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
Development

Successfully merging a pull request may close this issue.

7 participants