[GRASS-dev] Python and script header definitions for modules

[Huidae Cho]

Vaclav,

It's a great discussion. I *personally* think that enforcing Black or
Flake8 is less ideal [1], but coding consistency is always good, I agree.
Just one excerpt from [1]: "Autoformatters like Black are soulless, they
won't understand how each specific case will be most readable. This should
always be the developer's concern." Anyway, I consider it a necessary evil.

Option 3 is readily available with no changes in the core library, which is
great. It just takes mental effort to add the extra comment (# noqa: E501,
especially this number, I'll keep forgetting it). Does it even solve no
space between # and %, I hope?

Like option 5 as well, but can we embed YAML code as Python comments, not
as a separate file, which I don't like. Maybe, as option 6, we write a YAML
file and create a small utility that translates it to the current parser
format in option 3 and replace it in the Python script? Maybe, then, some
developers just write header definitions manually without YAML at all and
running the extra script is an additional burden.

Option 4 may not be too bad. We can just concatenate any lines with more
than a certain number (4?) of leading spaces to the current definition.

My biggest complaint is "#-space-%-space-key:-space-value". I just have to
type too many spaces manually. We could write a small script that handles
this, but again, that's an extra effort. Is it possible to create hooks so
we checkout "#%" and commit "#-space-%" automatically? Maybe, even long
definitions can be handled in the same way?

Best,
Huidae

[1]
https://luminousmen.com/post/my-unpopular-opinion-about-black-code-formatter

On Tue, Apr 20, 2021 at 11:11 PM Vaclav Petras <wenzeslaus at gmail.com> wrote:

> Dear all,
>
> After PR 1446, g.parser now accepts `# %` in addition to `#%` in order to
> allow following of the Python practice starting a block comment with `# `.
> This is checked by Flake8 E265. This resolved numerous warnings which you
> would otherwise get for each module when using Flake8 with default, that
> is, expected, settings. Consequently, users writing their own
> scripts/modules can use Flake8 with default settings without getting a
> flood of messages.
>
> However, not all issues were addressed by that. Many modules generate
> "E501 Line too long" warnings. Following default settings for the Black
> formater, we are now using 88 characters as line limit. Option and flag
> descriptions and the value of descriptions describing options* have often
> more characters than that.
>
> Currently, the E501 warning is disabled for all files in scripts and
> temporal directories and for selected `g.gui.*.py` files in gui/wxpython.
> However, we want to have the warning enabled globally. Although, Black
> takes care of most of the long lines, it does not touch some lines, namely
> long strings, and thus we want each file to be checked for Flake8 E501
> compliance.
>
> I would like to disable the check in each file just for the specific block
> of code, however, this is not possible because Flake8 does not allow
> disabling for blocks of code. Requiring the lines to be shorter won't work
> either because the descriptions item needs to be long. This leaves us with
> the following options:
>
> 1. Use per-file ignores to disable the warning and keep adding the files
> which need it. This makes the Flake8 configuration larger over time while
> our goal is to make it smaller over time. It also leaves the warning
> disabled for (other code in) these files.
>
> 2. Add inline Flake8 ignore comment to the offending line. This will make
> the line little longer, but it would be a good solution for normal Python
> code. However, in case of the script header, we would need to teach
> g.parser to understand trailing comments inside the relevant fields so that
> the Flake8 ignore comment does not leak into the user interface description.
>
> 3. According to the PEP 257 - Docstring Conventions document, the
> 'docstring of a script (a stand-alone program) should be usable as its
> "usage" message.' I don't think sticking something like our parser
> instructions into the docstring was what the authors had in mind.
> Additionally, it is not used like this either as far as I can tell.
> However, it would solve our issue. Just adding `"""` before the definition
> and adding `"""  # noqa: E501` after that disables the warning for the
> definition. The nice bonus is that we comply with PEP 257 by providing
> module docstring and by describing its interface there (in some way). The
> docstring presence is checked by Pylint's C0111 "Missing ... docstring".
>
> 4. We modify the parser so that at least some of the items can have
> multiple lines. However, the parser is currently quite line-oriented and
> the cost-benefit ratio may be low.
>
> 5. We change the script header definition format to some existing format
> that can break lines, i.e. allowing multi-line values. A clear candidate
> for the format is YAML or rather its simpler subset. This would have
> additional benefits of making the format a standard format. Which in turn
> would be beneficial for other things, e.g., for easier learning of the
> syntax. Combining this with option 3, we could drop the `# %` part to make
> the YAML more readily readable.
>
> Let me know what you think, what option you like, what you would add, and
> what you can help with.
>
> Thank you,
> Vaclav
>
>
>
> * Clearly, we should do something about the naming here, too!
>
>
> Example for option 3:
>
> """
> # %module
> # % description: Imports raster data into a GRASS raster map using GDAL
> library and reprojects on the fly.
> ...
> # %option
> # % key: resample
> # % type: string
> # % required: no
> # % multiple: no
> # % options:
> nearest,bilinear,bicubic,lanczos,bilinear_f,bicubic_f,lanczos_f
> # % description: Resampling method to use for reprojection
> # % descriptions: nearest;nearest neighbor;bilinear;bilinear
> interpolation;bicubic;bicubic interpolation;lanczos;lanczos
> filter;bilinear_f;bilinear interpolation with fallback;bicubic_f;bicubic
> interpolation with fallback;lanczos_f;lanczos filter with fallback
> # % answer: nearest
> # % guisection: Output
> # %end
> ...
> # %rules
> # % required: output,-e
> # %end
> """  # noqa: E501
>
> Links:
>
> https://github.com/OSGeo/grass/pull/1446
> https://www.flake8rules.com/rules/E265.html
> https://www.flake8rules.com/rules/E501.html
> http://pylint-messages.wikidot.com/messages:c0111
> _______________________________________________
> grass-dev mailing list
> grass-dev at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/grass-dev
>


-- 
Huidae Cho, Ph.D., GISP, /hidɛ t͡ɕo/, 조희대, 曺喜大
GRASS GIS Developer
https://idea.isnew.info/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20210613/bc942a44/attachment-0001.html>