lib/gis: solve write out conflicts of G_Percent/G_Progress when multithreading #24
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
cyliang368
changed the title
cyliang368
changed the title
The new ToolError tries to be even more aligned with CalledProcessError (by focusing on cmd and not tool and code like CalledModuleError), while providing messages more tailored towards Tools object and use cases. It also tries to polish further short one-line messages and distinguish between empty stderr and not captures stderr. API and documentation of it is more narrow and focused on the Tools use case specifically and includes type annotations. Document exception also in each tool page in the Parameters section followed by Returns: and Raises: blocks. The new Raises: block again mimics docs like NumPy (like the Returns: block), but with some liberty (it is in fact under section Parameters and it uses bold, emphasis and indent differently because of consistency with how we want the parameters to be represented and technical limitations related to indents which we do manually). The "tool" terminology now creates a consistent result in the doc (CalledModuleError was not on the tool pages because it would not be consistent).
The Docker test with Alpine is using scikit-learn, presuably to support the test of installing r.learn.ml2 (which in theory should install without it). This changes the test to use r.diversity instead of r.learn.ml2 which does not come with any dependency, reducing the setup and teardown phases, while still testing installation of a Python tool from grass-addons.
* address warnings * add OCI attributes; unify formating --------- Co-authored-by: ninsbl <stbl@nve.no> Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
The r.pack missed couple of the new PROJ-underscore files which are describing or defining the CRS (aka projection) at the project level. Specifically, projects created with EPSG have PROJ_SRID file (not PROJ_EPSG) which was previously omitted, and thus missing in the exported raster pack file. Additionally, the implementation missed another project-level information which is stored in and retrieved from the default computational region, namely projection and zone keys. The new code reads them with g.region and stores them in a JSON file inside the pack file. The new tests check for the expected files and check the format and content of the newly added JSON file.
Set the default role in Sphinx to literal so that backticks (not just double backticks) are interpreted as literal. Now they are interpreted as text in emphasis, but literal aligns that to Markdown use of backticks. While backticks mean a different thing in Sphinx/reStructuredText and Markdown, Sphinx can be configured to produce the same thing as a typical Markdown rendering, using the default role. While in another world, having the default role as `py:obj` or `any` would be an interesting option, with all the Markdown around us, having backticks interpreted as literal or code is most advantageous because it removes the issues when switching between Sphinx with reStructuredText for Python and Markdown everywhere else (Doxygen also supports Markdown). Relation to double backticks: The double backticks is unrelated to this in a sense that this changes how single backticks is interpreted (it changes the default role of it). Explicit roles before single backticks are not affected either. In other words, this is about roles, not about monospace/verbatim text. Current code - good case: As for the current code, my assumption is that if any current code is using single backticks now, it indeed meant to use it for some sort of monospace code rendering, so this is correcting it. Current code - bad case: The unset default role resulted in emphasis/italics in the current documentation. That would be appropriate for things like functions or objects. If describing an object was the intention of using single backticks (but explicit role `py:obj` was omitted), monospace is wrong, but not be a big mistake because it is still a code and it is still not linked anywhere (like before this change). In other words, linking symbols always required `py:obj` and proper way of doing emphasis/italics is not backticks, so these usages were wrong also before this change. From a search, these seem to be in minority, but there are some which are clearly meant as links to functions.
The create_project function (and the create_location function) specify in the documentation that it raises ScriptError on error. However, in some cases it calls fatal instead leading to system exit (SystemExit exception) instead with the default settings. This changes the fatal call into raise so that it aligns with the documentation and with other places in the function which raise ScriptError. This also makes it to behave in a more expected way in standard Python workflow. Current GRASS code already wraps the calls into try-except with ScriptError. In case of GUI, it checks the specific existence condition ahead of time. The exception text is not translatable according to the our new practice for exceptions. This also removes the warning when the project exists and overwrite is set to True. This is explicitly requested behavior, so warning not required. Additionally, while it kind of fit together with fatal with overwrite set to False, not it does not fit. Removing it also removes the need for a translatable string at that place which is often used with minimal setup during startup, so the result is simpler, less fragile code. Tests now cover all combinations cases (does not exist, exists, with and without overwrite).
For failed tests, the errors were printed twice, once by the GRASS gunittest class and once the Python unittest class. This removes the duplicated prints.
…#6358) * Fix Resource Leak Issue * Update raster/r.fill.dir/dopolys.c
Translated using Weblate (French) Currently translated at 34.2% (3452 of 10069 strings) Translated using Weblate (French) Currently translated at 34.2% (3445 of 10069 strings) Translated using Weblate (French) Currently translated at 96.8% (1796 of 1855 strings) Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translate-URL: https://weblate.osgeo.org/projects/grass-gis/grasslibs/ Translate-URL: https://weblate.osgeo.org/projects/grass-gis/grasslibs/fr/ Translate-URL: https://weblate.osgeo.org/projects/grass-gis/grassmods/ Translate-URL: https://weblate.osgeo.org/projects/grass-gis/grassmods/fr/ Translate-URL: https://weblate.osgeo.org/projects/grass-gis/grasswxpy/ Translation: GRASS GIS/grasslibs Translation: GRASS GIS/grassmods Translation: GRASS GIS/grasswxpy Co-authored-by: Weblate <noreply@weblate.org> Co-authored-by: Edouard Choiniere <echoix@users.noreply.weblate.osgeo.org>
* Fix Resource Leak * Changes
Co-authored-by: echoix <27212526+echoix@users.noreply.github.com>
Previously, the path with the package was required to exist (both the hardcoded etc/python under GISBASE and FHS variable one). Now, the code first attempts the import and only when the import fails, it tries to work out the path. The simple import should work when FHS is actually used and the grass package is where Python looks for packages (so no GRASS_PYDIR is needed there). It will also work when a user sets up PYTHONPATH manually expecting it will work the same as when setting it before running Python and importing the grass package there (both PYTHONPATH and GRASS_PYDIR will work, but PYTHONPATH will user the Python native mechanics to make the import work, while GRASS_PYDIR needs to use sys.path.append). If GRASS_PYDIR does not work (either the env variable or the one from the build), the code will attempt to find the GISBASE directory relative to the main executable and expects a non-FHS layout (which is reasonable since import should work without any setup with FHS). Prepend, not append to sys.path. Remove sys.path manipulations from the rest of grass.py file because the smarter code to do that is already executed by this point. * Try to import more specific grass.script, not just general grass to avoid having the import work for grass.py (issue on Windows where .py is not removed). If there is something which was imported as grass (grass.py would be the case), but does not have script, we need to convince Python to repeat the import of grass. We do that by removing it from sys.modules cache. This gives us a fresh start for next import with new path at the beginning of sys.path. Use a more specific exception type (available since Python 3.6). * Apply suggestions from code review --------- Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
* r.mapcalc: fix multiple outputs wih nprocs > 1
…ite3 and Python >= 3.12 (OSGeo#6692)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
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.
No description provided.