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

Support for pyproject.toml #71

Closed
Dekker1 opened this issue Sep 24, 2019 · 14 comments · Fixed by #75
Closed

Support for pyproject.toml #71

Dekker1 opened this issue Sep 24, 2019 · 14 comments · Fixed by #75

Comments

@Dekker1
Copy link

Dekker1 commented Sep 24, 2019

isort added support for using a pyproject.toml file for it's configuration in version 4.3.5. It would be great if this module could be compatible.

@sanjioh
Copy link
Contributor

sanjioh commented Nov 22, 2019

Strong +1 on this!

@gforcada
Copy link
Owner

For a few years I've asked for PyCQA/isort#574 which would make this little flake8 plugin's life so much easier, seems it is not getting any followup though 😕

@sanjioh
Copy link
Contributor

sanjioh commented Nov 22, 2019

Understood 🤔 didn’t noticed that.

In the meantime, would you be ok in just making flake8-isort depend on isort[pyproject] instead of on plain isort?
Would that solve the problem?

@gforcada
Copy link
Owner

@sanjioh I don't see what we would gain with adding dependencies on flake8-isort. If I got it right, what we are missing now is that projects can store their isort configuration on a pyproject.toml file which flake8-isort is not aware of.

We need to update https://github.com/gforcada/flake8-isort/blob/master/flake8_isort.py#L94

@sanjioh
Copy link
Contributor

sanjioh commented Nov 22, 2019

@gforcada It looks like isort picks up any config in pyproject.toml if present (and if toml is installed), without flake8-isort having to know.
To reproduce (using a virtualenv):

  • pyproject.toml
[tool.isort]
line_length = 10
  • test.py
from configparser import ConfigParser
  • pip install flake8 flake8-isort

  • without toml installed:

$ flake8 test.py --no-isort-config
test.py:1:1: F401 'configparser.ConfigParser' imported but unused
  • with toml installed (as per isort[pyproject] extra):
$ pip install toml
$ flake8 test.py --no-isort-config
test.py:1:1: I001 isort found an import in the wrong position
test.py:1:1: F401 'configparser.ConfigParser' imported but unused

Am I missing something? 🤔
Thanks for your replies 👍

@sanjioh
Copy link
Contributor

sanjioh commented Nov 27, 2019

@gforcada Hi! Any thoughts on this? Thanks!

@gforcada
Copy link
Owner

gforcada commented Dec 1, 2019

Oh, I see, despite that we pass where the configuration is, isort still looks for it and formats accordingly to that.

Then yes, 👍 on adding the dependency.

Now I'm curious if we actually have to do all the mambo jumbo to get the configuration, if anyway isort already looks for it 🤔

@sanjioh
Copy link
Contributor

sanjioh commented Dec 3, 2019

@gforcada Hi, I've opened a PR that adds the dependency and tests the new behaviour.
#75

Please let me know if that looks right to you or needs any fix! 👍

@mikewl
Copy link

mikewl commented Dec 24, 2019

This probably needs to be reopened.
I went through the code and did not see it acutally looking for the pyproject.toml file.

The original:

        # Check for '[isort]' section in other configuration files.
        for config_file in ('tox.ini', 'setup.cfg', '.flake8'):
            config_file_path = os.path.join(path, config_file)
            config = SafeConfigParser()
            config.read(config_file_path)
            if 'isort' in config.sections():
                return config_file_path

My modified one that finds the file:

        # Check for '[isort]' section in other configuration files.
        for config_file in ('tox.ini', 'setup.cfg', '.flake8', 'pyproject.toml'):
            config_file_path = os.path.join(path, config_file)
            config = SafeConfigParser()
            config.read(config_file_path)
            if 'isort' in config.sections():
                return config_file_path
            if 'tool.isort' in config.sections():
                return config_file_path

If I am understanding it right then all that is needed this side is the path to send to isort? There is no other logic done to the configuration?

If so I will happily make a PR for this. Still don't know what to do with the tests as they do pass with the previous behaviour as well.

@gforcada
Copy link
Owner

gforcada commented Jan 8, 2020

I totally missed this last comment, thanks @mikewl for it! A PR is always welcome, thanks in advance! 👍

@mikewl
Copy link

mikewl commented Jan 8, 2020

I will try getting to this over the weekend.

I need to spend some time figuring out why the tests were passing in the first place so that the tests fail appropriately on the current version too.

@rommguy
Copy link

rommguy commented Feb 2, 2020

Thank you so much for this awesome plugin.
I'm still getting "I002 no configuration found (.isort.cfg or [isort] in configs)" when I try to run flake8 and isort configuration is only in pyproject.toml
I'm using the isort 4.3.21 and flake8-isort 2.8.0
Thanks

@gforcada
Copy link
Owner

gforcada commented Feb 4, 2020

@rommguy you need isort 4.3.5 at least, there has been a pull request merged regarding that, but no new release so far.

jnns added a commit to jnns/flake8-isort that referenced this issue Feb 29, 2020
isort will look for a configuration in pyproject.toml
when the TOML library is available.

flake8-isort has its own mechanism to look up an appropriate
configuration file and will pass its path on to isort.
Right now, flake8-isort doesn't consider pyproject.toml files as
configuration file candidates and will raise an error if there's
no other configuration file than the pyproject.toml.

Luckily, there's the parameter `--no-isort-config` to make flake8-isort
not stall when it cannot find a config file.
In this case, isort will not be instructed to look for a specific
configuration file by flake8-isort but instead look for it itself
(beginning from either the currently linted file path
or the current working directory).

This is great because it means that we can rely on isort
to figure out which config to use
and linting behavior between both libraries will be exactly the same.
The search for a configuration file can be removed from flake8-isort
and we get Pyproject.toml support for free.
The `--no-isort-config` option can be removed as well
because from now on it will always be the burden of isort
to figure out which config to use.

Related to gforcada#71
Closes gforcada#78
jnns added a commit to jnns/flake8-isort that referenced this issue Feb 29, 2020
isort will look for a configuration in pyproject.toml
when the TOML library is available.

flake8-isort has its own mechanism to look up an appropriate
configuration file and will pass its path on to isort.
Right now, flake8-isort doesn't consider pyproject.toml files as
configuration file candidates and will raise an error if there's
no other configuration file than the pyproject.toml.

Luckily, there's the parameter `--no-isort-config` to make flake8-isort
not stall when it cannot find a config file.
In this case, isort will not be instructed to look for a specific
configuration file by flake8-isort but instead look for it itself
(beginning from either the currently linted file path
or the current working directory).

This is great because it means that we can rely on isort
to figure out which config to use
and linting behavior between both libraries will be exactly the same.
The search for a configuration file can be removed from flake8-isort
and we get Pyproject.toml support for free.
The `--no-isort-config` option can be removed as well
because from now on it will always be the burden of isort
to figure out which config to use.

Related to gforcada#71
Closes gforcada#78
@Luttik
Copy link

Luttik commented Apr 13, 2020

@gforcada I am having the same issue as @rommguy. (also with the same isort version and as far as I know 4.3.21 > 4.3.5)

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

Successfully merging a pull request may close this issue.

6 participants