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

Sort and format Python imports and enable isort check in CI #2883

Merged
merged 24 commits into from
Sep 19, 2023

Conversation

nicolossus
Copy link
Member

This PR sorts and formats imports in the Python codebase. The PR also introduces an isort runner to the CI and pre-commit.

The following files, in particular, should be reviewed closely:

  • pyproject.toml
  • .pre-commit-config.yml
  • ./github/workflows/nestbuildmatrix.yml

In order to have the nest import sorted properly in user facing examples, nest is marked as a known third-party in the isort configuration (by default isort recognizes that nest actually is first-party and will sort import nest in a separate block at the end of the import section).

@nicolossus nicolossus added T: Enhancement New functionality, model or documentation S: High Should be handled next I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Aug 9, 2023
@terhorstd terhorstd added this to To do in Build system and CI via automation Aug 22, 2023
Conflicts:
	doc/htmldoc/conf.py
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@nicolossus This looks neat for many cases and is a huge improvement, but as pointed out inline, but can we get more control about where our own stuff goes? I would like a three-block setup:

  1. Python Library modules
  2. Third-party modules
  3. import nest with all its variants

Is that feasible?

doc/htmldoc/networks/scripts/connections.py Show resolved Hide resolved
@heplesser
Copy link
Contributor

@nicolossus Could you update the PR to account for changes in master?

@nicolossus
Copy link
Member Author

@heplesser Yes, but did we reach a conclusion on where import nest should be placed in the sorting?

Conflicts:
	pynest/nest/lib/hl_api_spatial.py
	pynest/nest/lib/hl_api_types.py
	testsuite/pytests/sli2py_neurons/test_add_freeze_thaw.py
	testsuite/pytests/sli2py_neurons/test_neurons_handle_multiplicity.py
	testsuite/pytests/sli2py_recording/test_multimeter_stepping.py
	testsuite/pytests/test_NodeCollection.py
	testsuite/pytests/test_multimeter.py
	testsuite/pytests/test_weight_recorder.py
@nicolossus
Copy link
Member Author

@jessica-mitchell Could you take a look at the changed documentation?

Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

thanks lgtm!

@lekshmideepu lekshmideepu moved this from To do to In progress in Build system and CI Sep 15, 2023
Conflicts:
	doc/htmldoc/developer_space/guidelines/coding_guidelines_check.rst
@jougs jougs merged commit 5cce53d into nest:master Sep 19, 2023
14 checks passed
Build system and CI automation moved this from In progress to Done Sep 19, 2023
@nicolossus nicolossus deleted the sort_imports branch September 19, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Enhancement New functionality, model or documentation
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants