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 functionality to skip files during isort #196

Merged
merged 1 commit into from Jan 22, 2024
Merged

Conversation

jchalatu
Copy link
Contributor

@jchalatu jchalatu commented Jan 19, 2024

Motivation and context:

When running the static script, isort can run for an extremely long period of time if there are directories inside the project with large numbers of non-Python files (e.g. text files, binaries, etc.). Adding the ability to skip these directories/files with patterns alleviates this problem.

How has this been tested?

Added a new test and ran the entire suite locally.

How long should this take to review?

  • Quick (less than 40 lines changed or changes are straightforward)

Types of changes:

  • New feature (non-breaking change which adds functionality)
  • Non-code change (touches things like tests, documentation, build scripts)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • I have updated the documentation accordingly.
  • I have included a changelog entry.
  • I have added tests to cover my changes.
  • I have run the test suite locally and all tests passed.

@jchalatu jchalatu force-pushed the pyproject-exclusions branch 2 times, most recently from ac1eb68 to 35885f9 Compare January 19, 2024 19:09
Copy link

Coverage report

The coverage rate went from 100% to 100% ➡️

100% of new lines are covered.

Diff Coverage details (click to unfold)

nengo_bones/version.py

100% of new lines are covered (100% of the complete file).

@@ -23,6 +23,12 @@ force_exclude = '''
[tool.isort]
profile = "black"
src_paths = ["{{ pkg_name }}"]
{% if isort_exclude %}
skip_glob = [
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 we should use extend_skip_glob here so that it appends to the default exclusions rather than overwriting them.

While I was looking at the docs I also noticed the skip_gitignore option. I think we should probably enable that. That's the default behaviour for black, so that makes them consistent in that regard, and likely enabling that option would eliminate the need to use isort_exclude except in special circumstances.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay nice, I didn't see the extend_skip_glob option.

I was initially trying to use the skip_gitignore flag but I couldn't get it to find the .gitignore file in the project directory. Although, it's possible that the regular expression that was in the .gitignore for my use case wasn't compatible with what isort expects. I agree that it would be much cleaner to use the .gitignore rather than customizing the exclusions (I imagine all of the relevant exclusions would be found in the .gitignore anyways).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've pushed the fixup for adding the skip_gitignore flag but I'm still in the midst of seeing if I can get it to work in my use case. It works with extend_skip_glob at the moment, but ceases to work if I remove that from the config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah the problem is that skip_gitignore traverses and checks each file that satisfies the patterns in .gitignore and THEN skips them. Whereas, extend_skip_glob just bakes those patterns into the initial traversal. See this issue for reference PyCQA/isort#1912 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

Ah that's annoying. Hopefully they fix that at some point, but won't hurt to have the isort_exclude option anyway.

@drasmuss drasmuss force-pushed the pyproject-exclusions branch 2 times, most recently from 6608008 to 49eb386 Compare January 22, 2024 14:28
@drasmuss drasmuss merged commit d51fa91 into main Jan 22, 2024
12 checks passed
@drasmuss drasmuss deleted the pyproject-exclusions branch January 22, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants