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

Regression: isort enforces two newlines after imports in pyi stub file. #1284

Closed
jgerigmeyer opened this issue Jul 6, 2020 · 9 comments
Closed

Comments

@jgerigmeyer
Copy link

I think this is a regression of #942 in version 5.0.4, where black and isort disagree about the number of newlines after imports in a .pyi stub file:

from . import mark as mark

class _ExceptionInfo: ...

isort wants to change this to:

from . import mark as mark


class _ExceptionInfo: ...

My understanding from psf/black#837 and https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#stub-file-coding-style is that black is correct here?

isort settings:

[settings]
profile=black
@msapiro
Copy link

msapiro commented Jul 7, 2020

I have a similar issue which seems related. isort 5.0+ wants 2 blank lines between a from import within a def and a following comment. E.g.

    def on_email_deleted(self, email):
        from hyperkitty.tasks import rebuild_thread_cache_new_email


        # update or cleanup thread                  # noqa: E303 (isort issue)
        if self.emails.count() == 0:
            ...

is acceptable, but 0 or 1 blank lines between the from ...import and the following comment is not acceptable to isort, and more than 1 blank line is not acceptable to flake8. Thus the # noqa: to pass both.

This is not an issue if the first non-blank line following the import is not a comment. In that case, isort is happy with 0 or 1 blank lines.

Update: I just looked at b228c37 and it seems my issue is probably not related, so I'll open a new one.

@timothycrosley
Copy link
Member

Thank you for reporting!
I'm sorry this issue affected you! A fix has been released with version 5.0.5 of isort.

Thanks!

~Timothy

anoadragon453 added a commit to matrix-org/synapse that referenced this issue Jan 26, 2021
This new version no longer has the problem of adding/removing a blank line in `.pyi` files, which black disagrees with. This would cause `isort` to slightly modify `.pyi` files, before `black` would subsequently modify back directly afterwards.

Relevant `isort` issue: PyCQA/isort#1284
@lukaszmoroz
Copy link

Unfortunately, this issue still persists when the extension is not known, i.e. when the file is passed from the stdin (isort -). VSCode does that and it seems that in that case isort defaults to the .py mode. Would it be possible to pass the extension explicitly when run with the stdin?

@timothycrosley
Copy link
Member

Hi @lukaszmoroz, isort provides a way to pass along both the file name or just the extension when passing streaming content via the command line. See: https://pycqa.github.io/isort/docs/configuration/options.html#ext-format and https://pycqa.github.io/isort/docs/configuration/options.html#filename

@neersighted
Copy link

Hi @timothycrosley -- we're still seeing this issue in Poetry when invoking isort with pre-commit.

isort will incorrectly add two blank lines after imports in .pyi files, and black will change them to just one blank line. We are just using the default configuration. Any insight into if this is a regression, or if it's a config issue?

@timothycrosley
Copy link
Member

Hi @neersighted!

Sorry to see you are experiencing this issue. If you are talking about poetry itself, it is because the project has the lines_after_imports setting hard coded to 2 in it's config: https://github.com/python-poetry/poetry/blob/master/pyproject.toml#L79 As an aside, it is also currently pinned to an older version of isort in the precommit: https://github.com/python-poetry/poetry/blob/master/.pre-commit-config.yaml#L26

You could set up pre-commit to have a different config for pyi files than py files or remove the config line length override entirely to fix.

Let me know if I can help in any way.

~Timothy

@neersighted
Copy link

Hey @timothycrosley, thanks for replying so soon. Given your reply, would I be correct in assuming that this represents the correct solution?

i.e. use a command line argument to override our configured two lines after imports, only for .pyi files?

Thanks!

@timothycrosley
Copy link
Member

@neersighted, yes! That is probably the best for now, there's an improvement here for isort to make to enable changing certain config options based on extension. Right now all config options are shared per a run of isort, and this setting in particular is either automatically handled (in which case it does automatically handle pyi files correctly) or is statically set, in which case isort applies it the same to every extension.

@neersighted
Copy link

Okay, it makes sense now, thanks! Some sort of localized override (filetype, file extension, path, etc) in pyproject.toml seems like the ultimate solution. Until then, we'll just reset it back to default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants