OSGeo / grass-addons

GRASS GIS Addons Repository
https://grass.osgeo.org/grass-stable/manuals/addons/
GNU General Public License v2.0
103 stars 154 forks source link

i.sentinel.import and i.sentinel.mask: use of -j and json file is not documented #143

Open veroandreo opened 4 years ago

veroandreo commented 4 years ago

The addition of these options are a great improvement and I know the PR #137 is merged already, but how are we supposed to easily know the name of the json dump to use it as input for i.sentinel.mask? Do we need to browse in the mapset files or does it get printed when i.sentinel.import finishes?

Documentation of both new features in the respective manual pages would be great, too. Moreove, an extra example for the use case would be highly appreciated and would also make the improvement more visible and discoverable by users

Originally posted by @veroandreo in https://github.com/OSGeo/grass-addons/pull/137#issuecomment-613664101

neteler commented 4 years ago

The addition of these options are a great improvement and I know the PR #137 is merged already, but how are we supposed to easily know the name of the json dump to use it as input for i.sentinel.mask? Do we need to browse in the mapset files or does it get printed when i.sentinel.import finishes?

I agree that this needs to be documented. The underlying idea is that the history written by r.support is just textual (even quite limited in length and troublesome with the linebreaks) and cannot be easily machine-parsed. Hence additional JSON support was added to store the values in a better way.

Effectively, with -j a JSON file is written out for each band into LOCATION/MAPSET/json/BANDRASTERNAME.json, so nothing to fiddle with manually.

The advantage is that the user doesn't have to carry around the MTD_MSIL2A.xml from the zip'ed Sentinel-2 SAFE file as the relevant values are simply stored in the GRASS GIS data structure under json/.

This can then be fed to e.g. i.sentinel.mask as metadata parameter:

https://github.com/OSGeo/grass-addons/blob/master/grass7/imagery/i.sentinel/i.sentinel.mask/i.sentinel.mask.py#L122

OK, now we need to make it a text snippet to be added to the manual... :)

lucadelu commented 4 years ago

Effectively, with -j a JSON file is written out for each band into LOCATION/MAPSET/json/BANDRASTERNAME.json, so nothing to fiddle with manually.

@neteler @AnikaBettge from the source code it seems that json file are saved by default to LOCATION/MAPSET/cell_misc/MAP_NAME/description.json

anikaweinmann commented 4 years ago

@lucadelu yes you are right. I changed the description in https://github.com/OSGeo/grass-addons/pull/339

wenzeslaus commented 4 years ago

The underlying idea is that the history written by r.support is just textual (even quite limited in length and troublesome with the linebreaks) and cannot be easily machine-parsed. Hence additional JSON support was added to store the values in a better way.

This makes sense. LOCATION/MAPSET/cell_misc/MAP_NAME/description.json stated in a subsequent comment seems like a good choice.

Effectively, with -j a JSON file is written out for each band into LOCATION/MAPSET/json/BANDRASTERNAME.json, so nothing to fiddle with manually.

So why not to create that always and get rid of the -j flag and the trouble of documenting that (esp. since it is a very not standard behavior in GRASS).

As long a the path/name of that JSON file is good, the rest is relatively easy to make it backward/forward compatible: Just add a version to the top level. Then you can just say it is a new, perhaps experimental, part of the db/loc/mapset structure and threat that as anything else. Possibly add more modules or functionality which deal with that directly such as r.support. If you decide the contents/format is wrong, you just add checks for the version and change what is needed. If you are lucky, many changes in JSON will be just backward compatible automatically and if the filename contains extension (.json) then you could even change the format, although that seems unlikely here and now.

If you want to discuss this on mailing list, feel free to copy-paste what I just wrote.