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

Add zones in t.rast.univar #1474

Conversation

NikosAlexandris
Copy link
Member

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

    - Use open() instead of file()
    - Use a bytes-like object for ofile.write()
Best if a 'zones' options is integrated in the main function print_gridded_dataset_univar_statistics() and let the main code of t.rast.univar clean
@neteler
Copy link
Member

neteler commented Mar 21, 2021

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

@veroandreo
Copy link
Contributor

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?

@landam
Copy link
Member

landam commented Mar 21, 2021

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

You can convert PR into draft.

@NikosAlexandris NikosAlexandris marked this pull request as draft March 22, 2021 06:28
@NikosAlexandris
Copy link
Member Author

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.

@neteler
Copy link
Member

neteler commented Mar 29, 2021

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

@veroandreo
Copy link
Contributor

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?

@wenzeslaus
Copy link
Member

@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.

@neteler neteler added the enhancement New feature or request label Dec 9, 2021
@neteler neteler added this to the 8.2.0 milestone Dec 9, 2021
@wenzeslaus wenzeslaus modified the milestones: 8.2.0, 8.4.0 Feb 27, 2022
@ninsbl
Copy link
Member

ninsbl commented Sep 21, 2022

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

@ninsbl
Copy link
Member

ninsbl commented Oct 21, 2022

I am taking the liberty to close this one as it is superseded by #2588

@ninsbl ninsbl closed this Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants