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

conforming to flake8 appnexus #1395

Closed
JackDra opened this issue Aug 19, 2020 · 6 comments
Closed

conforming to flake8 appnexus #1395

JackDra opened this issue Aug 19, 2020 · 6 comments
Labels
bug Something isn't working question Further information is requested

Comments

@JackDra
Copy link

JackDra commented Aug 19, 2020

appnexus requires relative imports to have current directory above parent directory. E.g.

from .fileA import a_var
from ..fileB import b_var

but isort sorts them in order:

from ..fileB import b_var
from .fileA import a_var

Is there any way to reverse this condition? (i might have missed it in the docs somewhere...).

Currently it is reversed with the current tox.ini setup:

[flake8]
ignore = E123,E133,E226,E241,E262,E265,W503,W504
application-import-names = ...
application-package-names = ...
import-order-style = appnexus
exclude = build

[isort]
force_sort_within_sections = true
order_by_type = false
case_sensitive = false
multi_line_output = 5
sections=FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,APPLICATION,LOCALFOLDER
known_third_party = ...
known_first_party = ...
known_application = ...
lines_after_imports = 2
no_lines_before=LOCALFOLDER
@timothycrosley timothycrosley added the question Further information is requested label Aug 20, 2020
@timothycrosley
Copy link
Member

Hi @JackDra,

This is indeed supported by using the reverse_relative config option, see: https://timothycrosley.github.io/isort/docs/configuration/options/#reverse-relative.

Thanks!

~Timothy

@JackDra
Copy link
Author

JackDra commented Aug 20, 2020

Ah, I wasn't sure if that was for that.... My bad

It seems to still not be working. I found the issue to be a conflict(?) between force_sort_within_sections and reverse_relative.
If I have both on, it ignores reverse_relative. Is this just a limitation with the logic? or a bug?

@JackDra
Copy link
Author

JackDra commented Aug 20, 2020

From what I can tell, it seem like here:

https://github.com/timothycrosley/isort/blob/66bc89ff5290d28571b1d8af620644bcba5084f9/isort/output.py#L125-L134

The ordering of the '..' and '.' imports are flipped back to the regular ordering.

adding print statements to the sorting.naturally function called:

def naturally(to_sort: Iterable[str], key: Optional[Callable[[str], Any]] = None) -> List[str]:
    """Returns a naturally sorted list"""
    if key is None:
        key_callback = _natural_keys
    else:

        def key_callback(text: str) -> List[Any]:
            return _natural_keys(key(text))  # type: ignore

    show_sort = False
    for this_sort in to_sort:
        if any(['..' in ikey for ikey in to_sort]):
            show_sort = True
    if show_sort:
        print(to_sort)
        print(sorted(to_sort, key=key_callback))
        print()
    return sorted(to_sort, key=key_callback)

shows in the case where there are both . and .. imports:

OrderedDict([('..fields', OrderedDict([('UnitScalarField', True)])), ('..helpers', OrderedDict([('reserialize_date', True)])), ('..strict_schema', OrderedDict([('StrictSchema', True)])), ('.compound', OrderedDict([('BaseCompoundField', True)])), ('.compound_reagent', OrderedDict([('ICompoundReagentField', True)])), ('.compound_sample', OrderedDict([('CustomPHRField', True)])), ('.custom_fill_factor', OrderedDict([('CustomFillFactorField', True)]))])
['.compound', '.compound_reagent', '.compound_sample', '.custom_fill_factor', '..fields', '..helpers', '..strict_schema']


@timothycrosley timothycrosley added the bug Something isn't working label Aug 22, 2020
@timothycrosley
Copy link
Member

@JackDra,

Thanks for the additional information and to digging into the cause of the issue. I was able to reproduce, and can confirm that reverse_relative doesn't currently work when combined with force_sort_within_sections. I've fixed this in develop and it will make it's way out in the next release of isort. Additionally, a regression test has been added with your exact error case to ensure it is fixed for good.

Thanks!

~Timothy

@JackDra
Copy link
Author

JackDra commented Aug 22, 2020

Awsome, thanks!

I have put a request into our platform team to update our package manager (edm) with the latest isort version. I will tell them to hold off until the new release is out.

@timothycrosley
Copy link
Member

This change has just been deployed to PyPI in version 5.5.0

Thanks!

~Timothy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants