-
-
Notifications
You must be signed in to change notification settings - Fork 360
grass.experimental: Add name prefixes to Tools #5887
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
Draft
wenzeslaus
wants to merge
37
commits into
OSGeo:main
Choose a base branch
from
wenzeslaus:add-name-prefixes-to-tools-objects
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
grass.experimental: Add name prefixes to Tools #5887
wenzeslaus
wants to merge
37
commits into
OSGeo:main
from
wenzeslaus:add-name-prefixes-to-tools-objects
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds a Tools class which allows to access GRASS tools (modules) to be accessed using methods. Once an instance is created, calling a tool is calling a function (method) similarly to grass.jupyter.Map. Unlike grass.script, this does not require generic function name and unlike grass.pygrass module shortcuts, this does not require special objects to mimic the module families. Outputs are handled through a returned object which is result of automatic capture of outputs and can do conversions from known formats using properties. Usage example is in the _test() function in the file. The code is included under new grass.experimental package which allows merging the code even when further breaking changes are anticipated.
…ute with that stdin
…are different now)
…needs to be improved there). Allow usage through attributes, run with run_command syntax, and subprocess-like execution.
…est a tool when there is a close match for the name.
…tool in XY project, so useful only for things like g.extension or m.proj, but, with a significant workaround for argparse --help, it can do --help for a tool.
…that the type is float
…ng on Windows. Although the subprocess.run call is much nicer, whatever is necessary for the subprocess itself, good or bad, is contained in gs.Popen. This makes it consistent with the actual tool call later on. We may need a library wrapper for this in the future.
…run result, default to text mode
…workaround the tool help by manually handling the argparse help, but only for the run subcommand
… the error message which may get automatic suggestion by Python when __dir__ is now in place.
…me, count on Python possibly adding a sentence), use g.region rather than g.search.modules output in tests because the output is more predictable (although we loose the JSON test without format=json)
I like it and I'm up for that. However, if I would be the only user, I can live without that in order to make code cleaner and without unused features. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New feature or request
libraries
Python
Related code is in Python
tests
Related to Test Suite
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Name prefixes allows a Tools object to represent only a subset of tools with a common prefix. This provides functionality which is similar to grass.pygrass.modules.shortcuts, although the PR as it is implemented now adds really only the functionality to have things running that way and doesn't provide the actual objects to import. It may provide an alternative interface appealing to some users (grass.pygrass.modules.shortcuts is used in several addon tools and I know that @pesekon2 is using it). However, this would add another way of using it, making documentation and examples more challenging. In any case, this is an optional functionality which we can, but don't have to, add to tools. We can also add the functionality (what is in the PR now, but don't add any of the library objects like grass.pygrass.modules.shortcuts has).
The PR provides code which not only makes the individual tools usable this way, but also implements support for prefixes in attribute access and error messages when the tools are not found (showing the short method and the theoretical underlying tool).
I would be interested in opinions on whether to include this, how much to promote it, and whether to go further, and include "shortcut"/"prefix" objects into the library.
The PR currently build on top of #2923 (not merged yet), so the diff is large.
Examples
Basic example of use code. Creating a variable like this is totally a user choice, so it does not complicate the documentation of tools themselves. Unlike grass.pygrass.modules.shortcuts, this is actually more aligned with the basic case because only piece which is different is using
raster.mapcalc(...)
instead oftools.r_mapcalc(...)
(while instantiating Module from grass.pygrass is different than importing from grass.pygrass.modules.shortcuts).The variable name is again up to the user, similarly to using aliases for imports in grass.pygrass.modules.shortcuts, so using the short, original prefix is possible (I find this hard to understand actually because it breaks the Python expectations about what is a name and what is a operator - the Python operator
.
becomes part of the tool name). Anyway, this common way of using grass.pygrass.modules.shortcuts is possible:Just to make the point, that user can choose the variable name to fit whatever they need, here is another example:
What I find somewhat interesting, although without seeing any potential big of it, is using prefixes for sets of tools such as r.sim or r.stream, possibly tailoring the syntax to a specific use case:
Just like grass.pygrass.modules.shortcuts, shorter names suffer from a greater Python keyword conflict potential, so with prefixes active, the implementation allows trailing underscore for tool names so that things like v.import work:
As already mentioned above, the proposed functionality could be used in the library to create the tools to be used later which would make it really similar to grass.pygrass.modules.shortcuts):
However, a session would then need to be provided through an env parameter like now (while using a Tools object directly avoids that by Tools accepting a session parameter):