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

Add isort to sort imports alphabetically #745

Merged
merged 6 commits into from
Dec 22, 2020
Merged

Add isort to sort imports alphabetically #745

merged 6 commits into from
Dec 22, 2020

Conversation

seisman
Copy link
Member

@seisman seisman commented Dec 18, 2020

Description of proposed changes

This PR adds isort to automatically sort imports.

Notes:

  • profile = "black" is added to pyproject.toml for compatibility between black and isort, which requires isort>=5
  • pylint also depends on isort. Thus when we install pylint, isort is already installed.

References:

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Notes

  • You can write /format in the first line of a comment to lint the code automatically

@seisman seisman added the maintenance Boring but important stuff for the core devs label Dec 18, 2020
@seisman seisman requested a review from a team December 18, 2020 02:14
@seisman seisman marked this pull request as ready for review December 18, 2020 02:14
@seisman seisman added this to the 0.3.0 milestone Dec 18, 2020
@seisman seisman changed the title Add isort to sort imports Add isort to sort imports alphabetically Dec 18, 2020
Comment on lines +8 to +9
[tool.isort]
profile = "black"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used isort and quite like it, but I've found different isort versions to apply the sort styles rather inconsistently (specifically the standard lib/firstparty/thirdparty sections, see https://pycqa.github.io/isort/#custom-sections-and-ordering). Does this "black" profile keep things simple?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this "black" profile keep things simple?

No, profile = "black" is equivalent to the following settings:

multi_line_output: 3
include_trailing_comma: True
force_grid_wrap: 0
use_parentheses: True
ensure_newline_before_comments: True
line_length: 88

As for section orders, the default order (https://pycqa.github.io/isort/docs/configuration/options/#sections) is:

('FUTURE', 'STDLIB', 'THIRDPARTY', 'FIRSTPARTY', 'LOCALFOLDER')

Do you mean the default values change in different isort versions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was a point where they separated well known third-party libaries (e.g. numpy and pandas) from other third-party libraries. Maybe that has changed now with newer isort versions, but I'm not too sure.

Do you think we should add other configuration options? I'm looking at xarray's at pydata/xarray#4518 and they have some extra ones besides black.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will look at these options later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of our examples import some of the four packages: pygmt, numpy, pandas and xarray.

Currently, isort sorts them like

import numpy as np
import pandas as pd
import xarray as xr

import pygmt

because numpy, pandas, and xarray are known third-party packages, and pygmt is first-party package (not sure if I understand it correctly).

I don't think I like it. We can add options:

known_third_party = "pygmt"
force_to_top = "pygmt

so that they will be sorted like:

import pygmt
import numpy as np
import pandas as pd
import xarray as xr

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading through all the isort options, I added the following ones to pyproject.toml, similar to pydata/xarray#4518:

skip_gitignore = true
force_to_top = "pygmt"
known_third_party = "pygmt"

the only exception is default_section = THIRDPARTY, as I don't understand what it is, and adding it doesn't change the codes at all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to remove force_to_top, otherwise pylint will complain.

pygmt/__init__.py Outdated Show resolved Hide resolved
@seisman seisman merged commit 278e4e3 into master Dec 22, 2020
@seisman seisman deleted the isort branch December 22, 2020 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants