autoflake icon indicating copy to clipboard operation
autoflake copied to clipboard

load config from pyproject.toml or setup.cfg

Open akeeman opened this issue 3 years ago • 12 comments

Simple implementation for the currently missing functionality to load configuration from certain files:

  • [x] pyproject.toml
  • [x] setup.cfg (closes #68)

to do:

  • [x] pytests

akeeman avatar Sep 25 '20 13:09 akeeman

Coverage Status

Coverage decreased (-0.5%) to 98.803% when pulling bd4400c261dd35c132a36c32cd1b5f19162e95a2 on akeeman:patch-1 into d43d8a770c0f9ef2f68b368670ab882f6ca6ea03 on myint:master.

coveralls avatar Sep 25 '20 13:09 coveralls

@hakancelik96 Please review again

akeeman avatar Sep 28 '20 08:09 akeeman

Made the setup.cfg loader and possible other loaders simpler by parsing boolean values from string elsewhere and added correct syntax highlighting in README.rst.

@hakancelik96 Please review again

Note that the test failure is due to an unrelated feature break in Python 3.10.0a0

akeeman avatar Oct 06 '20 06:10 akeeman

@hakancelik96 any thoughts?

akeeman avatar Oct 16 '20 05:10 akeeman

@hakancelik96 any thoughts?

I am not the maintainer of this project, sorry. I just gave a little suggestion. I am developing a similar project named unimport

hakancelikdev avatar Oct 16 '20 07:10 hakancelikdev

@hakancelik96 I see. Thanks for sharing! I'll have a look.

@myint Have you got any feedback/other thoughts? I think that not having the ability to load configuration from a file is a real showstopper nowadays.

akeeman avatar Oct 16 '20 11:10 akeeman

@myint have you any feedback or time to merge?

akeeman avatar Nov 19 '20 14:11 akeeman

I'm sorry to ping you like this @myint. I know how open source can be energy draining and I definitely understand if you don't have time to look into this PR.

However, is it possible to know if I should go ahead and publish the changes of this PR under a different name on PyPI so that I can use it?

Thanks a lot!

mboutet avatar Aug 12 '21 21:08 mboutet

Is there just 1 person with merge permission for this repo? Maybe time to assign others? Seems like a waste of work not to get this in.

janosh avatar Apr 03 '22 11:04 janosh

@sigmavirus24 Any thoughts on who should be added as additional maintainers? I think the other PyCQA owners would have appropriate privileges now that I've moved this and docformatter to this org.

myint avatar Apr 10 '22 22:04 myint

@sigmavirus24 Any thoughts on who should be added as additional maintainers? I think the other PyCQA owners would have appropriate privileges now that I've moved this and docformatter to this org.

I haven't been involved in this project so I don't know which contributors should be invited. Other owners on PyCQA tend to not merge things across other projects they're not actively working on.

Are there other folks in PyCQA that have worked on these projects you'd like to invite?

Also, my response time is going to be super slow with a 3 week old that I'm caring for


Seems like a waste of work not to get this in.

On this topic, someone sent a pull request and did work, yes. I've not evaluated it, but just because someone did work, does not mean it should ever just be accepted or merged because they did it. Projects need to guard their boundaries more and more because maintainer burnout is an increasingly prevalant problem and throwing new people into the fire doesn't ever improve things.

All new feature work has to be carefully criticized else a project become so large, unruly, and complex that it's functionally unmaintainable.

sigmavirus24 avatar Apr 13 '22 14:04 sigmavirus24

Fwiw, the changes in this PR worked flawlessly for me.

pip install https://github.com/akeeman/autoflake/archive/patch-1.zip
autoflake **/*.py

Did nothing at first (as expected) but after adding the following to setup.cfg, it removed unused imports and variables.

# setup.cfg
[autoflake]
in_place = true
remove_unused_variables = true
remove_all_unused_imports = true
expand_star_imports = true
ignore_init_module_imports = true

The only thing I noticed (which I think is fine) is that only snake case is allowed. Some tools use param case in setup.cfg but trying

# setup.cfg
[autoflake]
in-place = true
remove-unused-variables = true
...

raised 'in-place' is not a valid configuration option.

janosh avatar Jun 08 '22 08:06 janosh

Rebased

akeeman avatar Aug 24 '22 11:08 akeeman

Let me fix https://github.com/PyCQA/autoflake/pull/79#issuecomment-1149640836 soon...

akeeman avatar Aug 24 '22 11:08 akeeman

Doesn't anyone know some configuration (file) abstraction layer? Because it's insane to write this logic over and over again for different libraries...

akeeman avatar Aug 24 '22 11:08 akeeman

@fsouza @akeeman I notice that the way this is implemented, values from config files take precedence over command line flags. That behaviour is surprising and at odds with the UNIX convention where the order of precedence is command line flags > environment variable > user config files > system config files. Was that a deliberate choice?

kynan avatar Oct 08 '22 13:10 kynan

@fsouza @akeeman I notice that the way this is implemented, values from config files take precedence over command line flags. That behaviour is surprising and at odds with the UNIX convention where the order of precedence is command line flags > environment variable > user config files > system config files. Was that a deliberate choice?

No, that one slipped. Sorry about that, let me send a fix.

fsouza avatar Oct 09 '22 02:10 fsouza

@kynan can you try #158?

fsouza avatar Oct 09 '22 03:10 fsouza