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

grass.script: Provide env parameter in the g.message API #3439

Merged
merged 2 commits into from
Mar 3, 2024

Conversation

wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented Feb 20, 2024

This makes the message, verbose and other functions consistent with other wrappers around run_command family calls. While not needed for multiple-mapset situations and parallelization, it is necessary when calls with messages are used without global environment being set and only a custom (local) environment is available which is the case in grass.script.setup (with #3438).

The PR aims at providing the interface, not updating all use cases (it will be applied for grass.script.setup in #3438).

This does not have any test since the current API does not allow for writing these test. This will be tested indirectly in the future (e.g. by #3438).

While connected to #3438, it is a separate issue more connected to the previous env parameter additions to run_command wrappers, so a separate PR and commit on main seems appropriate.

This makes the message, verbose and other functions consistent with other wrappers around run_command family calls. While not needed for multiple-mapset situations and parallelization, it is necessary when calls with messages are used without global environment being set and only custom environment available in grass.script.setup.

The PR aims at providing the interface, not updating all use cases (except the anticipated changes in grass.script.setup).

This does not have any test since the API for that is missing. This will be tested indirectly in the future.
petrasovaa
petrasovaa previously approved these changes Feb 28, 2024
…uple other function which were misleading or wrong.
@wenzeslaus
Copy link
Member Author

When I looked at this again, I realized these functions have per-parameter documentation, so I updated that as well. Please review.

@wenzeslaus wenzeslaus disabled auto-merge March 3, 2024 02:01
@wenzeslaus wenzeslaus enabled auto-merge (squash) March 3, 2024 02:02
@echoix
Copy link
Member

echoix commented Mar 3, 2024

There is only two non-doc changes, and simply passes down the env arg to run_command. Looks right

@wenzeslaus wenzeslaus merged commit 44ea597 into OSGeo:main Mar 3, 2024
25 checks passed
@wenzeslaus wenzeslaus deleted the env-for-messages branch March 3, 2024 03:10
@neteler neteler added this to the 8.4.0 milestone Mar 12, 2024
wenzeslaus added a commit to wenzeslaus/grass that referenced this pull request May 3, 2024
The original change in PR OSGeo#3439 has None instead of env passed to the error function in the fatal function. This passes the env parameter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants