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

handle FileSkipComment #230

Merged
merged 9 commits into from Oct 31, 2021
Merged

handle FileSkipComment #230

merged 9 commits into from Oct 31, 2021

Conversation

chrisdecker1201
Copy link
Contributor

@chrisdecker1201 chrisdecker1201 commented Oct 21, 2021

At the moment I've to face this issue because I added a skip_file comment to one of my files.

  • unit test
  • change log entry
Traceback (most recent call last):
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/isort/api.py", line 215, in sort_stream
    raise_on_skip=raise_on_skip,
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/isort/core.py", line 156, in process
    raise FileSkipComment("Passed in content")
isort.exceptions.FileSkipComment: Passed in content contains a file skip comment and was skipped.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/bin/darker", line 8, in <module>
    sys.exit(main_with_error_handling())
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/darker/__main__.py", line 365, in main_with_error_handling
    return main()
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/darker/__main__.py", line 348, in main
    report_unmodified=output_mode == OutputMode.CONTENT,
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/darker/__main__.py", line 76, in format_edited_parts
    black_config.get("line_length"),
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/darker/import_sorting.py", line 72, in apply_isort
    isort_code(code=content.string, **isort_args),
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/isort/api.py", line 99, in sort_code_string
    show_diff=show_diff,
  File "/home/chris/.local/share/virtualenvs/timeslime-IMN5r0BP/lib/python3.7/site-packages/isort/api.py", line 218, in sort_stream
    raise FileSkipComment(content_source)
isort.exceptions.FileSkipComment: Passed in content contains a file skip comment and was skipped.

This may fix it.

Copy link
Owner

@akaihola akaihola left a comment

Choose a reason for hiding this comment

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

Great patch! A couple of minor fixes still needed.

@akaihola akaihola added the enhancement New feature or request label Oct 26, 2021
@akaihola akaihola added this to the 1.4.0 milestone Oct 26, 2021
@chrisdecker1201
Copy link
Contributor Author

chrisdecker1201 commented Oct 27, 2021

@akaihola Hey :). I don't understand why the tests are failing. Can you help me?

I think isort is failing because it's detecting the comment in the test

isort.exceptions.FileSkipComment: Passed in content contains a file skip comment and was skipped.

isort is handeling every line with # isort: skip_file regardless if this is a string and no comment.

https://github.com/PyCQA/isort/blob/80c213b2d5d95350ce5dc0d7bbc4dc0065435bd9/isort/core.py#L153

And for the rest maybe you have an idea?

@akaihola
Copy link
Owner

@chrisdecker1201, there are two test failures.

The first one is

AttributeError: '_io.StringIO' object has no attribute 'buffer'

and is caused by a change in Flake 4.0.0. I'm downgrading to Flake 3.x in #233 – could you review that one? After it's merged, you could rebase on master and that should fix the problem.

The other failure is just due to Black reformatting. Add a second empty line before def test_isort_file_skip_comment(): and the test should pass.

@chrisdecker1201
Copy link
Contributor Author

Thank you for your help. I think when the fix in isort is integrated than everything could be right now :)

Copy link
Owner

@akaihola akaihola left a comment

Choose a reason for hiding this comment

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

A change log entry is still needed, otherwise I think this is ready to be merged if we get the tests to pass.

Copy link
Owner

@akaihola akaihola left a comment

Choose a reason for hiding this comment

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

Let's fix test suite compatibility with current and older isort.

CHANGES.rst Outdated Show resolved Hide resolved
Copy link
Owner

@akaihola akaihola left a comment

Choose a reason for hiding this comment

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

This is good!

Next, I'll rebase this on master and merge for inclusion in the 1.4.0 release.

@sourcery-ai
Copy link

sourcery-ai bot commented Oct 31, 2021

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 0.88%.

Quality metrics Before After Change
Complexity 0.93 ⭐ 1.29 ⭐ 0.36 👎
Method Length 40.44 ⭐ 40.30 ⭐ -0.14 👍
Working memory 5.22 ⭐ 5.27 ⭐ 0.05 👎
Quality 85.84% 84.96% -0.88% 👎
Other metrics Before After Change
Lines 151 168 17
Changed files Quality Before Quality After Quality Change
src/darker/import_sorting.py 87.98% ⭐ 84.42% ⭐ -3.56% 👎
src/darker/tests/test_import_sorting.py 83.69% ⭐ 85.49% ⭐ 1.80% 👍

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

@akaihola akaihola merged commit 59ff71d into akaihola:master Oct 31, 2021
@chrisdecker1201 chrisdecker1201 deleted the feature/file_skip_comment branch October 31, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants