Skip to content

Use isort for standardizing import formatting #23048

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
alimcmaster1 opened this issue Oct 8, 2018 · 2 comments · Fixed by #23096
Closed

Use isort for standardizing import formatting #23048

alimcmaster1 opened this issue Oct 8, 2018 · 2 comments · Fixed by #23096
Labels
CI Continuous Integration Clean Code Style Code style, linting, code_checks
Milestone

Comments

@alimcmaster1
Copy link
Member

alimcmaster1 commented Oct 8, 2018

@TomAugspurger kindly pointed out on this PR that we could be using isort for formatting imports and have a standardized approach across the codebase.

I will draft up an initial config based on the rules used in dask-ml and the comments from @jbrockmendel in the above PR. If anyone feels strongly about any of the rules please comment here.

My proposed plan for introducing is we define a config and I run this for a handful of files. ( 1st PR )

We then over the period of a few weeks we encourage developers to run isort over their PRs and we can iron out any issues in the config set up.

Then we introduce a check into our CI build isort --recursive --check-only. And perhaps use this so developers can just run flake8 on their code like before.

Any thoughts/concerns here?

@TomAugspurger
Copy link
Contributor

+1 on the general idea of standardizing.

I don't have any preferences on the style of the imports, beyond separating

  • stdlib
  • 3rd party
  • pandas

into three sections.

In the past we've always used absolute imports. I find relative imports slightly nicer to work with. If we want to change to relative, now is the time to do it.

On the process, I think the first step (after agreeing on a config) is to add the isort check to our CI config. We'll need to set the skip setting. At the start this will be a long list of files, but over the course of several PRs we can work that down and maybe eventually eliminate it entirely (there may be some files we don't care about).

@jbrockmendel
Copy link
Member

@alimcmaster1 if its convenient to configure isort to order pandas-internal imports by dependency structure that'd be great, but don't go too far out of your way. I think I'm the only one who makes a point of it.

@datapythonista datapythonista added CI Continuous Integration Code Style Code style, linting, code_checks Clean labels Oct 9, 2018
This was referenced Oct 9, 2018
@jreback jreback added this to the 0.24.0 milestone Oct 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Clean Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants