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

Breaking change to configuration logic in v2.1.0 #251

Open
kevinr-electric opened this issue Apr 17, 2023 · 8 comments
Open

Breaking change to configuration logic in v2.1.0 #251

kevinr-electric opened this issue Apr 17, 2023 · 8 comments
Labels

Comments

@kevinr-electric
Copy link

By changing the config file resolution for .toml files, this new minor version has broken configuration in cases where the --config CLI argument is used.

In the new version's changes, in the merge_configuration_file function on line 1199 autoflake now calls the process_pyproject_toml function for .toml files instead the process_config_file function. The result is that the tool will now try to get the "tool.autoflake" section section instead of the "autoflake" section. In other words, a configuration file that worked with v2.0.2 breaks on v2.1.0 (and vice versa).

As an aside, is there a place where release notes are posted? I couldn't find them on GH or Pypi.

@fsouza fsouza added the bug label Apr 17, 2023
@fsouza
Copy link
Collaborator

fsouza commented Apr 18, 2023

Hi @kevinr-electric, thanks for reporting. I'm going to revert that change and then we can have a separate flag that's exclusive for the location of pyproject.toml.

Currently, autoflake doesn't keep a changelog in a dedicated location. We can introduce something with GitHub releases though.

fsouza added a commit that referenced this issue Apr 18, 2023
This introduced a breaking change (#251).

This reverts commit 780b44f.
@fsouza fsouza pinned this issue Apr 18, 2023
@kevinr-electric
Copy link
Author

Hey @fsouza, thanks for the update. A releases section with release notes would also be appreciated!

@bricker
Copy link
Contributor

bricker commented May 4, 2023

I'm confused here; Passing a toml file to --config was completely broken before (that was the whole point of this PR). How were you doing that?

@joeyagreco
Copy link

In version 2.1.1, I'm seeing that [autoflake] works, but [tool.autoflake] does not for a pyproject.toml file.

This is in direct contradiction to the PyPi docs

@fsouza
Copy link
Collaborator

fsouza commented May 29, 2023

@joeyagreco can you share an example? I can't repro it:

% cat pyproject.toml
[project]
name = "sample"
description = "Just a sample"
license = { text = "ISC" }

[tool.autoflake]
expand-star-imports = true
% cat sample.py
import os
from sys import *

print("hi", file=stderr)
% autoflake sample.py
--- original/sample.py
+++ fixed/sample.py
@@ -1,4 +1,3 @@
-import os
-from sys import *
+from sys import stderr

 print("hi", file=stderr)
% autoflake --version
autoflake 2.1.1

@joeyagreco
Copy link

joeyagreco commented May 29, 2023

Sure! @fsouza

Here's my pyproject.toml file

[autoflake]
ignore_init_module_imports = true
in_place = true
recursive = true
remove_all_unused_imports = true

Check version

% autoflake --version
> autoflake 2.1.1

Run it

% autoflake --config=pyproject.toml .
> 

Works (removes an unused import)

Let's change the pyproject.toml

[tool.autoflake]
ignore_init_module_imports = true
in_place = true
recursive = true
remove_all_unused_imports = true

Now let's run it again

% autoflake --config=pyproject.toml .
> can't parse config file 'C:\foo\baz\pyproject.toml'

@fsouza
Copy link
Collaborator

fsouza commented May 29, 2023

@joeyagreco ahh ok, yes --config doesn't support pyproject.toml at the moment. It's being parsed as an ini file, and toml just happens to be compatible with that.

@fsouza
Copy link
Collaborator

fsouza commented May 29, 2023

I'm confused here; Passing a toml file to --config was completely broken before (that was the whole point of #249). How were you doing that?

Sorry, I had missed this message, but I assume people were relying on the fact that simple toml files can be parsed as ini files. After thinking about this a little bit, I feel like we can roll #249 forward again. Alternatively, we could have a dedicated flag for pyproject.toml.

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

No branches or pull requests

4 participants