grass icon indicating copy to clipboard operation
grass copied to clipboard

Add zones in t.rast.univar

Open NikosAlexandris opened this issue 3 years ago • 8 comments

This is NOT a real pull request. Rather a prototype idea. Not extensively tested, no real tests written.

NikosAlexandris avatar Mar 21 '21 01:03 NikosAlexandris

(would you mind to separate out 5e00142 (Quick-fix for d.correlate.py) into another PR?)

neteler avatar Mar 21 '21 07:03 neteler

Sounds interesting, Nikos :)

AFAIU, this "non-PR" PR adds r.univar's zones functionality to t.rast.univar. Does it use only one raster map as zones or a time series of zones? If the first case, how does it differ from the add-on v.strds.stats that estimates zonal stats for a vector of areas?

I'm in for testing! Would you mind providing an example?

veroandreo avatar Mar 21 '21 16:03 veroandreo

This is NOT a real pull request. Rather a prototype idea. Not extensively tested, no real tests written.

You can convert PR into draft.

landam avatar Mar 21 '21 22:03 landam

Sounds interesting, Nikos :)

AFAIU, this "non-PR" PR adds r.univar's zones functionality to t.rast.univar. Does it use only one raster map as zones or a time series of zones? If the first case, how does it differ from the add-on v.strds.stats that estimates zonal stats for a vector of areas?

Only one (integer CELL) categorical or thematic (you name it) map, exactly as in r.univar. It looks it's like in v.strds.stats, though I don't remember if and when I have used it.

I'm in for testing! Would you mind providing an example?

Just run a normal t.rast.univar command and provide a (land cover?) map with zones=LandCoverMap. Later on I can copy-paste a concrete example.

NikosAlexandris avatar Mar 22 '21 06:03 NikosAlexandris

The Travis test shows:

python3 -t -m py_compile /home/travis/build/OSGeo/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/temporal_topology_dataset_connector.py

  File "/home/travis/build/OSGeo/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/univar_statistics.py", line 185

    gscript.message(_(f'Zone {category} \'{label}\''))

                                                   ^
SyntaxError: invalid syntax

neteler avatar Mar 29 '21 06:03 neteler

The Travis test shows:

python3 -t -m py_compile /home/travis/build/OSGeo/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/temporal_topology_dataset_connector.py

  File "/home/travis/build/OSGeo/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/univar_statistics.py", line 185

    gscript.message(_(f'Zone {category} \'{label}\''))

                                                   ^
SyntaxError: invalid syntax

Maybe something like:

gscript.message(_("Zone {} {}").format(category,label))

works?

veroandreo avatar Mar 29 '21 19:03 veroandreo

@veroandreo is right. Some more tips for _(f'Zone {category} \'{label}\''):

  • Do no use f-strings with translatable strings, i.e., no _(f"..."). There is no solution yet to combine f-strings with gettext. Note that in case of _("...").format(...)), the format function runs on the translated string, not on the string literal. This is not checked on Python level, so it is important to remember that.
  • Black style: Use double quotes for strings by default. To avoid escaping quotes in strings, use the other quotes for the string itself (so e.g. ("a: 'label'" or 'a: "label"')). This PR is opened against 7.8, but in master branch the code complies with Black and the CI checks that.
  • Use {} only with one parameter. Use "{a} {b}".format(a=a, b=b) with multiple variables or "{a} {b}".format(**locals()) if you want to avoid spelling out the names. Potentially, the "{a} {b}".format(a=a, b=b) might be better long term as the translatable strings is fully independent on the variable names. However, you can achieve the same with "{a} {b}".format(**locals()) if you turn it into "{a} {b}".format(a=x, b=y) later when you rename the variables which is fine too.

wenzeslaus avatar Mar 29 '21 20:03 wenzeslaus

@NikosAlexandris OK to close this in favor of #2588 ?

ninsbl avatar Sep 21 '22 18:09 ninsbl

I am taking the liberty to close this one as it is superseded by https://github.com/OSGeo/grass/pull/2588

ninsbl avatar Oct 21 '22 18:10 ninsbl