setuptools_scm icon indicating copy to clipboard operation
setuptools_scm copied to clipboard

Auto-generated write_to file triggering errors in flake8, pydocstyle, and pylint

Open jamesbraza opened this issue 5 years ago • 7 comments

I use the write_to option in my pyproject.toml config file to write to a Python __version__.py file. It creates a file that looks like this:

# coding: utf-8
# file generated by setuptools_scm
# don't change, don't track in version control
version = '0.1.dev41+ge5eee1b.d20201024'

flake8

I run flake8 (PEP8 linter) on my package with the flake8-black plugin for black.

Unfortunately, flake8 fails with this message:

__version__.py:4:11: BLK100 Black would make changes.

I have since fixed things on my side using the exclude configuration option for flake8.


pydocstyle

The same is true for pydocstyle:

__version__.py:1 at module level:
        D100: Missing docstring in public module

I think we can ignore pydocstyle, it seems superfluous to have auto-generated files be given module-level docstrings.


pylint

And also for pylint:

__version__.py:4:0: C0103: Constant name "version" doesn't conform to UPPER_CASE naming style (invalid-name)

I can work around the pylint "invalid-name" error using the ignore configuration option.


The Request

Can the auto-generated files be made compatible with PEP8 (and by the transitive property, black)?

jamesbraza avatar Oct 24 '20 02:10 jamesbraza

Per the pydocstyle "D100: Missing docstring in public module" error being raised, I learned from reading pydocstyle's Exclude given files #186 of using flake8-docstrings to ignore docstring problems on a per-file basis.

My issue still stands on the PEP8 front.

jamesbraza avatar Oct 24 '20 02:10 jamesbraza

We can add a better template, but it needs a transition period/plan , as it is a breaking change

RonnyPfannschmidt avatar Oct 24 '20 05:10 RonnyPfannschmidt

To review:

  • pydocstyle: let's ignore that I said this... I don't agree with adding a module-level docstring
  • pylint: let's also ignore this... making the variable name say VERSION as opposed version is definitely a breaking change, and adds little value besides adherence to a convention

And as I mentioned in the original issue, it's easy to add ignores of these in a config file.


Per being compatible with Black, the way to fix it is the distinction between ' and ".

Just changing the version string to be written with double quotes "0.1.dev41+ge5eee1b.d20201024" as opposed to single quotes '0.1.dev41+ge5eee1b.d20201024' would satisfy.

It seems the debate between ' and " is fierce and controversial:


I am not sure why changing from ' to " would be a breaking change.

As I said in the original question, I am definitely not blocked by this, there are easy workarounds. I just thought it was worth voicing.

It's entirely up to you if you choose to move forward with this or not. Cheers!

jamesbraza avatar Oct 24 '20 20:10 jamesbraza

if this is only about the black detail, then its a minor change and fine to do

RonnyPfannschmidt avatar Oct 24 '20 20:10 RonnyPfannschmidt

Honestly, I wouldn't submit to formatters. In particular, because different options exist, as well as different preferences. What if somebody would want to go for blue or something different becomes popular. It's unsubstantial trying to chase a new thing repeatedly.

The PEP 8 related bits, OTOH, make sense to me. A module-level docstring, for example, is a part of the DX. It'd show up in editors and REPLs. I'd also, maybe, include type annotations into the template, in order to improve the DX. Although, I tend to just add a .pyi stub file into projects so that the type checkers don't go crazy if I don't run packaging machinery that would put the actual .py file in place...

For things like different variable names, I'd probably document in README that these must be disabled in the linter configs, or maybe just included # pylint: disable=invalid-name right into the template. Auto-generated files should never be linted/formatted in every end-user project, FWIW — this defeats the whole point of not caring how said file is managed...

webknjaz avatar Mar 27 '23 14:03 webknjaz

Personally I like the idea of creating a black safe template simply to remove frustrations for beginners

As string formatting is used i have a idea for a fix

RonnyPfannschmidt avatar Mar 27 '23 15:03 RonnyPfannschmidt

With version 8.0.3, an automatically created version file looks like this:

# file generated by setuptools_scm
# don't change, don't track in version control
TYPE_CHECKING = False
if TYPE_CHECKING:
    from typing import Tuple

__version__ = version = '0.11.10.post2+g030a308'  # type: str
__version_tuple__ = version_tuple = (0, 11, 10, 'g030a308')  # type: Tuple[int | str, ...]

For me, this triggers the following errors when running flake8:

dcnum/_version.py:5:5: F401 'typing.Tuple' imported but unused
dcnum/_version.py:8:80: E501 line too long (90 > 79 characters)

An easy workaround would be to add # flake8: noqa comment:

# flake8: noqa
# file generated by setuptools_scm
# don't change, don't track in version control
TYPE_CHECKING = False
if TYPE_CHECKING:
    from typing import Tuple

__version__ = version = '0.11.10.post2+g030a308'  # type: str
__version_tuple__ = version_tuple = (0, 11, 10, 'g030a308')  # type: Tuple[int | str, ...]

Then flake8 would not complain anymore. https://flake8.pycqa.org/en/3.1.1/user/ignoring-errors.html#ignoring-entire-files

I would second @webknjaz here for not letting anyone worry about auto-generated files.

paulmueller avatar Sep 28 '23 21:09 paulmueller