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

Rename setup.cfg to .flake8 for more obviousness. #2959

Closed
wants to merge 1 commit into from

Conversation

mo-g
Copy link

@mo-g mo-g commented Feb 15, 2018

As suggested by @pp-mo in issue #2941. This is purely a naming convention change, and has no practical effect. Especially since we're not yet running flake8 via stickler yet.

@SciTools-assistant SciTools-assistant added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Feb 15, 2018
@stickler-ci
Copy link

Could not review pull request. It may be too large, or contain no reviewable changes.

@mo-g
Copy link
Author

mo-g commented Feb 15, 2018

I also hereby accept the CLA and will mail you a signed copy when I next unbury my printer cabinet.

Hmm. May have to add the printer to the 'take to Cambridge' list.

@ajdawson
Copy link
Member

mail you a signed copy when I next unbury my printer cabinet.

Emailing an electronically signed version would be faster... 📮 😢

@mo-g
Copy link
Author

mo-g commented Feb 15, 2018

Well, assuming 'typing my name' is a sufficient electronic signature, that's done. I don't currently have an active PGP key or S/MIME delegation to sign it with, so...

@QuLogic
Copy link
Member

QuLogic commented Feb 15, 2018

Not sure about this as other tools, like pytest, would also read from this file. So it's one file for all tools vs many (possibly hidden) files for every single tool.

@pp-mo
Copy link
Member

pp-mo commented Feb 16, 2018

one file for all tools vs many (possibly hidden) files for every single tool.

Well yes, but only tools that are explicitly needed or supported by the project. How many could there be?

I just think "setup.cfg" is a dumb name really.
And it only works for those tools that "agree to share" this magic single filename : where is that consensus ?
So, what is the scope of that thing supposed to be ?
I'd be happy with this if it were covered by a PEP,
but I don't find that ...

According to my crude web searches, the original intent of setup.cfg is configuration for installation,
in fact = default options for commands in setup.py (thus a distutils concept).
This : https://docs.python.org/3.6/distutils/configfile.html?highlight=setup%20cfg
(but that is legacy + cannot find an up-to-date account)

On the other side of the argument I find this : http://renesd.blogspot.co.uk/2017/02/setupcfg-solution-to-python-config-file.html

@mo-g
Copy link
Author

mo-g commented Feb 16, 2018

Ok, there are clearly some good arguments both ways here.

Looking at that blog post by @illume, there's a few tools to look at. My quick google-fu didn't find any more obvious ones, so I'll treat that list as canonical without further data. I don't see this project using mypy any time soon, nor replacing unittests with pytest. So... from where I'm standing the only other tool we'd be likely to consider using is coverage.py. If anyone wants to suggest we deploy that, sure - but in the mean time if flake8 is the only thing using the file - the .flake8 name does seem to be more obvious.

TL;DR - I'm with Patrick on this one, till we use more tools that share the config.

Edit: Would also support there being a PEP for this.

Edit2: According to the tox project, pytest are already planning to deprecate setup.cfg anyway. (tox-dev/tox#297)

@ajdawson
Copy link
Member

There is a valid point that we only have one tool to configure, and therefore one specific configuration file seems appropriate. However, we could be a little forward thinking here and not make extra work for ourselves...

We currently use nose for testing, which is deprecated; it is likely that we will follow cartopy's lead and switch to pytest at some point. When we do that we could add another configuration file, or we could continue to use setup.cfg (by the way, pytest is not deprecating/removing the use of setup.cfg, they are deprecating/removing the section title [pytest] within setup.cfg in favour of [tool:pytest]).

It seems to me that whilst changing the name of this file now is perfectly fine, it is quite likely we'll want to change it back again quite soon.

@mo-g
Copy link
Author

mo-g commented Feb 19, 2018

@pp-mo What do you think? If it's likely we're going to be using more tools, the argument for using a standard file does make sense. On the other hand, if that second tool isn't exactly round the corner, then the clarity of the different filename also makes sense for the meantime.

@pp-mo
Copy link
Member

pp-mo commented Feb 19, 2018

What do you think?

Personally I still prefer this approach, but given all the opinion against I wouldn't be happy merging it, so status quo wins.

@mo-g mo-g closed this Feb 19, 2018
@mo-g mo-g deleted the flake_config branch February 19, 2018 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants