t.rast.seasons: new temporal module to get seasonally aggregated data #920
Conversation
veroandreo
changed the title
|
@lucadelu you are trying to merge into grass7 branch, should go into grass8 I believe |
As I said in other pull request on my production system I still have GRASS 7. :-( |
Co-authored-by: Veronica Andreo <[email protected]>
t.rast.aggregate.ds already return a strds for each year, but it make sense to have one with all the maps together. Going to implement it |
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| options, flags = gscript.parser() |
There was a problem hiding this comment.
While this is in the old examples, unless you need options and flags as globals (which is a bad practice anyway), there is no reason to have this here. It should be in main.
|
|
||
| if __name__ == "__main__": | ||
| options, flags = gscript.parser() | ||
| sys.exit(main()) |
There was a problem hiding this comment.
Similar to above, unless there main is returning a return code, no reason to pass it to sys.exit. Just put main() here.
| from datetime import datetime | ||
|
|
||
| import grass.temporal as tgis | ||
| import grass.script as gscript |
There was a problem hiding this comment.
While less unique, in new code we mostly adopted import grass.script as gs. grass.script is used as lot, so it can be short. It is not a requirement, so if there would be potential for confusion in a particular file, it can gscript.
| else: | ||
| out_strds = basename | ||
|
|
||
| seasons_name = ["spring", "summer", "autumn", "winter"] |
There was a problem hiding this comment.
This is list, so plural would make sense season_names or seasons. The loop variable seas can then be an actual word like season_name or season.
| if strds.find("@") >= 0: | ||
| id_ = strds | ||
| else: | ||
| id_ = strds + "@" + gscript.gisenv()["MAPSET"] |
| process_queue = pymod.ParallelModuleQueue(int(nprocs)) | ||
|
|
||
| # create t.rast.aggregate.ds module to be copied | ||
| mod = pymod.Module("t.rast.aggregate.ds") |
There was a problem hiding this comment.
Documentation says:
Setting run_ to False is important, otherwise a parallel processing is not possible
| for year in years: | ||
| remod = pymod.Module("t.remove") | ||
| remod.inputs.inputs = f"sample_seasons_{year}@{mapset}" | ||
| remod.flags.r = True | ||
| remod.flags.f = True | ||
| remod.flags.quiet = True | ||
| if output_name: | ||
| listmaps = pymod.Module("t.rast.list") | ||
| listmaps.inputs.input = f"{out_strds_{year}}" | ||
| listmaps.inputs.columns = ["name"] |
There was a problem hiding this comment.
This should be a cleanup function registered with atexit to avoid a prematurely terminated process leaving data behind.
| out_maps = [] | ||
| # remove space time vector datasets | ||
| for year in years: | ||
| remod = pymod.Module("t.remove") |
There was a problem hiding this comment.
Isn't the default for run_ True? See https://grass.osgeo.org/grass78/manuals/libpython/_modules/pygrass/modules/interface/module.html#Module
There was a problem hiding this comment.
Or is that only when other parameters are supplied? (Then disregard my comment about parallel processing.)
@lucadelu can we switch the base to |
Ok, I updated the script to be work properly in GRASS8 not I'm going to rebase it |
|
@wenzeslaus I updated the code according to you suggestions, thanks a lot. Running the script it works properly, but now I have a problem compiling the source code to run the tests It remains stuck at this point, do you have any suggestions? |
My fault, there was a |
Does this mean that you need it (only?) in GRASS7? And then someone to forward-port it to GRASS8 in a new PR? Just to understand. |
I don't know where the "not" is coming from :-) However I tried to rebase and I got some errors, so I'll close this pull request and I'll do new one based on GRASS 8 branch |
|
replaced by #1003 |
This new addons run t.rast.aggregate.ds with a temporal dataset of season and it is able to get seasonal aggregate values