Use # %, not #% in Python scripts

[wenzeslaus]

Dear all,

I prepared a change to g.parser and related code which changes the option definitions in scripts (aka script header) from:

#%option

#% key

#%end

to:

# %option

# % key

# %end

The reason is PEP8 compliance where "each line of a block comment starts with a # and a single space" [2].

The PR is removing all use of #% from the code, but it is still allowed, so addons and existing user code should continue to work. However, I don't see a need to depreciate support of #% at this point.

This change moves us a little closer to PEP8 or Flake8 compliance. We can now enable the check in CI once the normal comments are fixed. (The file header comment is ignored, so no need to change that, but we can consider that anyway.)

The vision is that users and contributors can write scripts which just work with more or less default Flake8 settings. This is a big step towards that because it removes tens of warnings for a standard module.

Vaclav

[1] https://github.com/OSGeo/grass/pull/1287

[2] https://www.python.org/dev/peps/pep-0008/#block-comments

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev

[Markus Metz-3]

On Fri, Feb 5, 2021 at 5:14 AM Vaclav Petras <[hidden email]> wrote:

>
> Dear all,
>
> I prepared a change to g.parser and related code which changes the option definitions in scripts (aka script header) from:
>
> #%option
> #% key
> #%end
>
> to:
>
> # %option
> # % key
> # %end
>
> The reason is PEP8 compliance where "each line of a block comment starts with a # and a single space" [2].
>
> The PR is removing all use of #% from the code, but it is still allowed, so addons and existing user code should continue to work. However, I don't see a need to depreciate support of #% at this point.
>
> This change moves us a little closer to PEP8 or Flake8 compliance. We can now enable the check in CI once the normal comments are fixed. (The file header comment is ignored, so no need to change that, but we can consider that anyway.)
>
> The vision is that users and contributors can write scripts which just work with more or less default Flake8 settings. This is a big step towards that because it removes tens of warnings for a standard module.

These warnings for #% are a real annoyance, distracting from meaningful warnings. The suggested changes will make code quality checking and code improvement much easier.

+1

Markus M

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev

[wenzeslaus]

On Fri, Feb 5, 2021 at 2:35 PM Markus Metz <[hidden email]> wrote:

On Fri, Feb 5, 2021 at 5:14 AM Vaclav Petras <[hidden email]> wrote:

>
> Dear all,
>
> I prepared a change to g.parser and related code which changes the option definitions in scripts (aka script header) from:
>
> #%option
> #% key
> #%end
>
> to:
>
> # %option
> # % key
> # %end
>
> The reason is PEP8 compliance where "each line of a block comment starts with a # and a single space" [2].
>
> The PR is removing all use of #% from the code, but it is still allowed, so addons and existing user code should continue to work. However, I don't see a need to depreciate support of #% at this point.
>
> This change moves us a little closer to PEP8 or Flake8 compliance. We can now enable the check in CI once the normal comments are fixed. (The file header comment is ignored, so no need to change that, but we can consider that anyway.)
>
> The vision is that users and contributors can write scripts which just work with more or less default Flake8 settings. This is a big step towards that because it removes tens of warnings for a standard module.

These warnings for #% are a real annoyance, distracting from meaningful warnings. The suggested changes will make code quality checking and code improvement much easier.

+1

Thanks for the feedback. The change is now merged.

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev

[Markus Neteler]

On Fri, Feb 5, 2021 at 5:14 AM Vaclav Petras <[hidden email]> wrote:

>
> Dear all,
>
> I prepared a change to g.parser and related code which changes the option definitions in scripts (aka script header) from:
>
> #%option
> #% key
> #%end
>
> to:
>
> # %option
> # % key
> # %end
>
> The reason is PEP8 compliance where "each line of a block comment starts with a # and a single space" [2].
>
> The PR is removing all use of #% from the code, but it is still allowed, so addons and existing user code should continue to work. However, I don't see a need to depreciate support of #% at this point.
>
> This change moves us a little closer to PEP8 or Flake8 compliance. We can now enable the check in CI once the normal comments are fixed. (The file header comment is ignored, so no need to change that, but we can consider that anyway.)
>
> The vision is that users and contributors can write scripts which just work with more or less default Flake8 settings. This is a big step towards that because it removes tens of warnings for a standard module.

Do you plan to apply the same to the grass7/ addons?
This would be great for consistency.

Markus

> Vaclav
>
> [1] https://github.com/OSGeo/grass/pull/1287
> [2] https://www.python.org/dev/peps/pep-0008/#block-comments
> _______________________________________________
> grass-dev mailing list
> [hidden email]
> https://lists.osgeo.org/mailman/listinfo/grass-dev
_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev

[wenzeslaus]

On Thu, Feb 18, 2021 at 4:11 PM Markus Neteler <[hidden email]> wrote:

On Fri, Feb 5, 2021 at 5:14 AM Vaclav Petras <[hidden email]> wrote:

> The vision is that users and contributors can write scripts which just work with more or less default Flake8 settings. This is a big step towards that because it removes tens of warnings for a standard module.

Do you plan to apply the same to the grass7/ addons?
This would be great for consistency.

Yes, it would. Putting it to grass7/ would, however, require backport because that's what v7.8 is using. After we separate addons for v8, sure. The '# %' does not look like something for backport, maybe after it really matures, but that means time.

 

Markus

> Vaclav
>
> [1] https://github.com/OSGeo/grass/pull/1287
> [2] https://www.python.org/dev/peps/pep-0008/#block-comments
> _______________________________________________
> grass-dev mailing list
> [hidden email]
> https://lists.osgeo.org/mailman/listinfo/grass-dev

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev