From 3e5d84fe77a0be436a9153e966b25774006ca0a8 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 29 Nov 2024 04:21:08 +0000 Subject: [PATCH 1/3] CI(deps): Update ruff to v0.8.1 --- .github/workflows/python-code-quality.yml | 2 +- .pre-commit-config.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-code-quality.yml b/.github/workflows/python-code-quality.yml index feb6ba70c9d..c3d877a3df1 100644 --- a/.github/workflows/python-code-quality.yml +++ b/.github/workflows/python-code-quality.yml @@ -36,7 +36,7 @@ jobs: # renovate: datasource=pypi depName=bandit BANDIT_VERSION: "1.8.0" # renovate: datasource=pypi depName=ruff - RUFF_VERSION: "0.8.0" + RUFF_VERSION: "0.8.1" runs-on: ${{ matrix.os }} permissions: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a486f2ee0da..39d38f17008 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,7 @@ repos: ) - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.8.0 + rev: v0.8.1 hooks: # Run the linter. - id: ruff From 70fd6374530c82c39f6680aa98c8db19e59e6351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edouard=20Choini=C3=A8re?= <27212526+echoix@users.noreply.github.com> Date: Fri, 29 Nov 2024 23:47:53 +0000 Subject: [PATCH 2/3] style: Fix RUF055 [*] Plain string pattern passed to `re` function --- gui/wxpython/photo2image/ip2i_manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gui/wxpython/photo2image/ip2i_manager.py b/gui/wxpython/photo2image/ip2i_manager.py index e3832c681b4..d720e0a5adc 100644 --- a/gui/wxpython/photo2image/ip2i_manager.py +++ b/gui/wxpython/photo2image/ip2i_manager.py @@ -426,14 +426,13 @@ def __init__( GMessage(_("A POINTS file exists, renaming it to POINTS_BAK")) # """Make a POINTS file """ - import re try: fc = open(self.file["camera"]) fc_count = 0 for line in fc: fc_count += 1 - if re.search(r"NUM", line): + if "NUM" in line: storeLine = fc_count numberOfFiducial = int(line.split()[-1]) dataFiducialX = [] From 98af2b5bf75d18741f9fbfec12f0df5ef4b42029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edouard=20Choini=C3=A8re?= <27212526+echoix@users.noreply.github.com> Date: Sat, 30 Nov 2024 14:31:52 +0000 Subject: [PATCH 3/3] style: Fix or ignore os-listdir (PTH208) Use `pathlib.Path.iterdir()` instead. Ruff rule: https://docs.astral.sh/ruff/rules/os-listdir/ --- man/build_manual_gallery.py | 13 +++-- pyproject.toml | 31 +++++++++--- python/grass/grassdb/checks.py | 91 ++++++++++++++++++---------------- scripts/r.pack/r.pack.py | 6 +-- 4 files changed, 85 insertions(+), 56 deletions(-) diff --git a/man/build_manual_gallery.py b/man/build_manual_gallery.py index 2fa6ace609d..a0f8176a246 100755 --- a/man/build_manual_gallery.py +++ b/man/build_manual_gallery.py @@ -12,11 +12,16 @@ # for details. # ############################################################################# +from __future__ import annotations import os from pathlib import Path import fnmatch import re +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterable img_extensions = ["png", "jpg", "gif"] img_patterns = ["*." + extension for extension in img_extensions] @@ -88,7 +93,7 @@ """ -def img_in_file(filename, imagename, ext) -> bool: +def img_in_file(filename: str | os.PathLike[str], imagename: str, ext: str) -> bool: # for some reason, calling search just once is much faster # than calling it on every line (time is spent in _compile) if ext == "html": @@ -99,7 +104,7 @@ def img_in_file(filename, imagename, ext) -> bool: return bool(re.search(pattern, Path(filename).read_text())) -def file_matches(filename, patterns): +def file_matches(filename: str, patterns: Iterable[str]): return any(fnmatch.fnmatch(filename, pattern) for pattern in patterns) @@ -163,11 +168,11 @@ def main(ext): continue if file_matches(filename, img_patterns): for man_filename in man_files: - if img_in_file(os.path.join(man_dir, man_filename), filename, ext): + if img_in_file(Path(man_dir, man_filename), filename, ext): img_files[filename] = man_filename # for now suppose one image per manual filename - with open(os.path.join(man_dir, output_name), "w") as output: + with open(Path(man_dir, output_name), "w") as output: output.write( header1_tmpl.substitute( title="GRASS GIS %s Reference Manual: Manual gallery" % grass_version diff --git a/pyproject.toml b/pyproject.toml index 85ad3e1283e..f14ec703ce2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -268,14 +268,20 @@ ignore = [ "gui/**" = ["PLW0108"] # See https://github.com/OSGeo/grass/issues/4124 "gui/wxpython/animation/temporal_manager.py" = ["SIM115"] "gui/wxpython/core/*.py" = ["SIM115"] +"gui/wxpython/core/globalvar.py" = ["PTH208"] +"gui/wxpython/core/settings.py" = ["PTH208"] +"gui/wxpython/datacatalog/catalog.py" = ["PTH208"] "gui/wxpython/dbmgr/base.py" = ["SIM115"] -"gui/wxpython/gcp/manager.py" = ["SIM115"] +"gui/wxpython/gcp/manager.py" = ["PTH208", "SIM115"] "gui/wxpython/gmodeler/*.py" = ["SIM115"] "gui/wxpython/gui_core/*.py" = ["SIM115"] +"gui/wxpython/gui_core/dialogs.py" = ["PTH208"] "gui/wxpython/iclass/frame*.py" = ["SIM115"] "gui/wxpython/iclass/frame.py" = ["FLY002"] "gui/wxpython/iclass/statistics.py" = ["A005"] +"gui/wxpython/icons/grass_icons.py" = ["PTH208"] "gui/wxpython/image2target/*.py" = ["SIM115"] +"gui/wxpython/image2target/ii2t_manager.py" = ["PTH208"] "gui/wxpython/iscatt/plots.py" = ["PLW0108"] "gui/wxpython/lmgr/workspace.py" = ["SIM115"] "gui/wxpython/location_wizard/wizard.py" = ["SIM115"] @@ -284,9 +290,11 @@ ignore = [ "gui/wxpython/modules/mcalc_builder.py" = ["SIM115"] "gui/wxpython/photo2image/*.py" = ["SIM115"] "gui/wxpython/psmap/*.py" = ["SIM115"] +"gui/wxpython/psmap/dialogs.py" = ["PTH208"] "gui/wxpython/psmap/utils.py" = ["PGH004"] "gui/wxpython/rdigit/controller.py" = ["SIM115"] "gui/wxpython/rlisetup/*.py" = ["SIM115"] +"gui/wxpython/rlisetup/frame.py" = ["PTH208"] "gui/wxpython/timeline/frame.py" = ["FLY002"] "gui/wxpython/tools/update_menudata.py" = ["SIM115"] "gui/wxpython/tplot/frame.py" = ["FLY002"] @@ -299,31 +307,39 @@ ignore = [ "lib/imagery/testsuite/test_imagery_signature_management.py" = ["SIM115"] "lib/imagery/testsuite/test_imagery_sigsetfile.py" = ["FURB152"] "lib/init/grass.py" = ["SIM115"] +"lib/init/testsuite/test_grass_tmp_mapset.py" = ["PTH208"] "locale/grass_po_stats.py" = ["SIM115"] +"man/build.py" = ["PTH208"] +"man/build_class_graphical.py" = ["PTH208"] +"man/build_manual_gallery.py" = ["PTH208"] +"man/build_rest.py" = ["PTH208"] "python/grass/__init__.py" = ["PYI056"] "python/grass/exp*/tests/grass_script_mapset_session_test.py" = ["SIM117"] "python/grass/exp*/tests/grass_script_tmp_mapset_session_test.py" = ["SIM117"] "python/grass/gunittest/case.py" = ["PT009"] -"python/grass/gunittest/loader.py" = ["PYI024"] +"python/grass/gunittest/loader.py" = ["PTH208", "PYI024"] "python/grass/gunittest/multireport.py" = ["PYI024"] "python/grass/gunittest/testsu*/d*/s*/s*/subsub*/t*/test_segfaut.py" = ["B018"] "python/grass/gunittest/testsuite/test_assertions_rast3d.py" = ["FLY002"] "python/grass/imaging/images2*.py" = ["SIM115"] +"python/grass/imaging/images2ims.py" = ["PTH208"] "python/grass/jupyter/testsuite/interactivemap_test.py" = ["PGH004"] "python/grass/jupyter/testsuite/map_test.py" = ["PGH004"] "python/grass/pydispatch/signal.py" = ["A005"] -"python/grass/pygrass/modules/grid/grid.py" = ["SIM115"] +"python/grass/pygrass/gis/__init__.py" = ["PTH208"] +"python/grass/pygrass/modules/grid/grid.py" = ["PTH208", "SIM115"] "python/grass/pygrass/modules/grid/testsuite/test_*_modules_grid_doctests.py" = ["F401"] "python/grass/pygrass/modules/interface/env.py" = ["SIM115"] "python/grass/pygrass/modules/testsuite/test_pygrass_modules_doctests.py" = ["F401"] "python/grass/pygrass/raster/category.py" = ["FURB189"] "python/grass/pygrass/raster/segment.py" = ["SIM115"] "python/grass/pygrass/tests/*.py" = ["SIM115"] +"python/grass/pygrass/utils.py" = ["PTH208"] "python/grass/pygrass/vector/geometry.py" = ["PYI024"] "python/grass/pygrass/vector/sql.py" = ["FLY002"] "python/grass/pygrass/vector/testsuite/test_table.py" = ["PLW0108"] "python/grass/script/array.py" = ["A005"] -"python/grass/script/core.py" = ["SIM115"] +"python/grass/script/core.py" = ["PTH208", "SIM115"] "python/grass/script/db.py" = ["SIM115"] "python/grass/script/raster.py" = ["SIM115"] "python/grass/script/utils.py" = ["FURB189", "SIM115"] @@ -332,7 +348,7 @@ ignore = [ "python/grass/temporal/stds_export.py" = ["SIM115"] "python/grass/temporal/stds_import.py" = ["SIM115"] "python/grass/temporal/univar_statistics.py" = ["SIM115"] -"python/grass/utils/download.py" = ["SIM115"] +"python/grass/utils/download.py" = ["PTH208", "SIM115"] "raster/r.*/testsuite/*.py" = ["SIM115"] "raster/r.topidx/*.py" = ["SIM115"] "raster3d/r3.flow/testsuite/r3flow_test.py" = ["FLY002"] @@ -343,8 +359,10 @@ ignore = [ "scripts/db.in.ogr/db.in.ogr.py" = ["SIM115"] "scripts/db.test/db.test.py" = ["SIM115"] "scripts/db.univar/db.univar.py" = ["SIM115"] +"scripts/g.download.project/g.download.project.py" = ["PTH208"] "scripts/g.extension.all/g.extension.all.py" = ["SIM115"] -"scripts/g.extension/g.extension.py" = ["SIM115"] +"scripts/g.extension/g.extension.py" = ["PTH208", "SIM115"] +"scripts/g.extension/testsuite/test_addons_modules.py" = ["PTH208"] "scripts/g.search.modules/g.search.modules.py" = ["SIM115"] "scripts/i.in.spotvgt/i.in.spotvgt.py" = ["SIM115"] "scripts/i.oif/i.oif*.py" = ["SIM115"] @@ -370,6 +388,7 @@ ignore = [ "temporal/t.unregister/t.unregister.py" = ["SIM115"] "utils/**.py" = ["SIM115"] "utils/generate_release_notes.py" = ["PGH004"] +"utils/thumbnails.py" = ["PTH208"] "vector/v.fill.holes/examples.ipynb" = ["PTH201"] [tool.ruff.lint.flake8-import-conventions.extend-aliases] diff --git a/python/grass/grassdb/checks.py b/python/grass/grassdb/checks.py index 369cdf60e62..9024ac33517 100644 --- a/python/grass/grassdb/checks.py +++ b/python/grass/grassdb/checks.py @@ -9,6 +9,8 @@ .. sectionauthor:: Vaclav Petras """ +from __future__ import annotations + import datetime import glob import os @@ -20,7 +22,7 @@ from grass.script import gisenv -def mapset_exists(path, location=None, mapset=None): +def mapset_exists(path: str | os.PathLike[str], location=None, mapset=None) -> bool: """Returns True whether mapset path exists. Either only *path* is provided or all three parameters need to be provided. @@ -30,27 +32,27 @@ def mapset_exists(path, location=None, mapset=None): :param mapset: name of a Mapset if not part of *path* """ if location and mapset: - path = os.path.join(path, location, mapset) + path = Path(path, location, mapset) elif location or mapset: raise ValueError(_("Provide only path or all three parameters, not two")) - return os.path.exists(path) + return Path(path).exists() -def location_exists(path, location=None): +def location_exists(path: str | os.PathLike[str], location=None) -> bool: """Returns True whether location path exists. :param path: Path to a Location or to a GRASS GIS database directory :param location: name of a Location if not part of *path* """ if location: - path = os.path.join(path, location) - return os.path.exists(path) + path = Path(path, location) + return Path(path).exists() # TODO: distinguish between valid for getting maps and usable as current # https://lists.osgeo.org/pipermail/grass-dev/2016-September/082317.html # interface created according to the current usage -def is_mapset_valid(path, location=None, mapset=None): +def is_mapset_valid(path: str | os.PathLike[str], location=None, mapset=None) -> bool: """Return True if GRASS Mapset is valid Either only *path* is provided or all three parameters need to be provided. @@ -64,13 +66,13 @@ def is_mapset_valid(path, location=None, mapset=None): # WIND doesn't exist (assuming that neither GRASS_REGION nor # WIND_OVERRIDE environmental variables are set). if location and mapset: - path = os.path.join(path, location, mapset) + path = Path(path, location, mapset) elif location or mapset: raise ValueError(_("Provide only path or all three parameters, not two")) - return os.access(os.path.join(path, "WIND"), os.R_OK) + return os.access(Path(path, "WIND"), os.R_OK) -def is_location_valid(path, location=None): +def is_location_valid(path: str | os.PathLike[str], location=None) -> bool: """Return True if GRASS Location is valid :param path: Path to a Location or to a GRASS GIS database directory @@ -81,8 +83,8 @@ def is_location_valid(path, location=None): # containing a PERMANENT/DEFAULT_WIND file is probably a GRASS # location, while a directory lacking it probably isn't. if location: - path = os.path.join(path, location) - return os.access(os.path.join(path, "PERMANENT", "DEFAULT_WIND"), os.F_OK) + path = Path(path, location) + return os.access(Path(path, "PERMANENT", "DEFAULT_WIND"), os.F_OK) def is_mapset_current(database, location, mapset) -> bool: @@ -101,7 +103,7 @@ def is_location_current(database, location) -> bool: return bool(database == genv["GISDBASE"] and location == genv["LOCATION_NAME"]) -def is_current_user_mapset_owner(mapset_path): +def is_current_user_mapset_owner(mapset_path: str | os.PathLike[str]) -> bool: """Returns True if mapset owner is the current user. On Windows it always returns True.""" # Note that this does account for libgis built with SKIP_MAPSET_OWN_CHK @@ -118,12 +120,12 @@ def is_current_user_mapset_owner(mapset_path): return mapset_uid == os.getuid() -def is_different_mapset_owner(mapset_path): +def is_different_mapset_owner(mapset_path: str | os.PathLike[str]) -> bool: """Returns True if mapset owner is different from the current user""" return not is_current_user_mapset_owner(mapset_path) -def get_mapset_owner(mapset_path): +def get_mapset_owner(mapset_path: str | os.PathLike[str]) -> str | None: """Returns mapset owner name or None if owner name unknown. On Windows it always returns None.""" if sys.platform == "win32": @@ -164,27 +166,27 @@ def is_first_time_user(): return False -def is_mapset_locked(mapset_path): +def is_mapset_locked(mapset_path: str | os.PathLike[str]) -> bool: """Check if the mapset is locked""" lock_name = ".gislock" - lockfile = os.path.join(mapset_path, lock_name) - return os.path.exists(lockfile) + lockfile = Path(mapset_path, lock_name) + return lockfile.exists() -def get_lockfile_if_present(database, location, mapset): +def get_lockfile_if_present(database, location, mapset) -> str | None: """Return path to lock if present, None otherwise Returns the path as a string or None if nothing was found, so the return value can be used to test if the lock is present. """ lock_name = ".gislock" - lockfile = os.path.join(database, location, mapset, lock_name) - if os.path.isfile(lockfile): - return lockfile + lockfile = Path(database, location, mapset, lock_name) + if lockfile.is_file(): + return str(lockfile) return None -def get_mapset_lock_info(mapset_path): +def get_mapset_lock_info(mapset_path: str | os.PathLike[str]): """Get information about .gislock file. Assumes lock file exists, use is_mapset_locked to find out. Returns information as a dictionary with keys @@ -233,13 +235,14 @@ def get_reason_id_mapset_not_usable(mapset_path): return None -def dir_contains_location(path): +def dir_contains_location(path: str | os.PathLike[str]) -> bool: """Return True if directory *path* contains a valid location""" - if not os.path.isdir(path): + p = Path(path) + if not p.is_dir(): return False - for name in os.listdir(path): - if os.path.isdir(os.path.join(path, name)): - if is_location_valid(path, name): + for name in p.iterdir(): + if name.is_dir(): + if is_location_valid(name): return True return False @@ -260,8 +263,8 @@ def get_mapset_invalid_reason(database, location, mapset, none_for_no_reason=Fal # Since we are trying to get the one most likely message, we need all # those return statements here. # pylint: disable=too-many-return-statements - location_path = os.path.join(database, location) - mapset_path = os.path.join(location_path, mapset) + location_path = Path(database, location) + mapset_path = location_path / mapset # first checking the location validity # perhaps a special set of checks with different messages mentioning mapset # will be needed instead of the same set of messages used for location @@ -271,14 +274,14 @@ def get_mapset_invalid_reason(database, location, mapset, none_for_no_reason=Fal if location_msg: return location_msg # if location is valid, check mapset - if mapset not in os.listdir(location_path): + if not mapset_path.exists(): # TODO: remove the grass.py specific wording return _( "Mapset <{mapset}> doesn't exist in GRASS Location <{location}>" ).format(mapset=mapset, location=location) - if not os.path.isdir(mapset_path): + if not mapset_path.is_dir(): return _("<%s> is not a GRASS Mapset because it is not a directory") % mapset - if not os.path.isfile(os.path.join(mapset_path, "WIND")): + if not (mapset_path / "WIND").is_file(): return ( _( "<%s> is not a valid GRASS Mapset" @@ -287,7 +290,7 @@ def get_mapset_invalid_reason(database, location, mapset, none_for_no_reason=Fal % mapset ) # based on the is_mapset_valid() function - if not os.access(os.path.join(mapset_path, "WIND"), os.R_OK): + if not os.access(mapset_path / "WIND", os.R_OK): return ( _( "<%s> is not a valid GRASS Mapset" @@ -303,7 +306,9 @@ def get_mapset_invalid_reason(database, location, mapset, none_for_no_reason=Fal ).format(mapset=mapset, location=location) -def get_location_invalid_reason(database, location, none_for_no_reason=False): +def get_location_invalid_reason( + database, location, none_for_no_reason=False +) -> str | None: """Returns a message describing what is wrong with the Location The goal is to provide the most suitable error message @@ -321,14 +326,14 @@ def get_location_invalid_reason(database, location, none_for_no_reason=False): :param none_for_no_reason: When True, return None when reason is unknown :returns: translated message or None """ - location_path = os.path.join(database, location) - permanent_path = os.path.join(location_path, "PERMANENT") + location_path = Path(database, location) + permanent_path = location_path / "PERMANENT" # directory - if not os.path.exists(location_path): + if not location_path.exists(): return _("Location <%s> doesn't exist") % location_path # permanent mapset - if "PERMANENT" not in os.listdir(location_path): + if not permanent_path.exists(): return ( _( "<%s> is not a valid GRASS Location" @@ -336,7 +341,7 @@ def get_location_invalid_reason(database, location, none_for_no_reason=False): ) % location_path ) - if not os.path.isdir(permanent_path): + if not permanent_path.is_dir(): return ( _( "<%s> is not a valid GRASS Location" @@ -345,7 +350,7 @@ def get_location_invalid_reason(database, location, none_for_no_reason=False): % location_path ) # partially based on the is_location_valid() function - if not os.path.isfile(os.path.join(permanent_path, "DEFAULT_WIND")): + if not (permanent_path / "DEFAULT_WIND").is_file(): return ( _( "<%s> is not a valid GRASS Location" @@ -362,14 +367,14 @@ def get_location_invalid_reason(database, location, none_for_no_reason=False): ) -def get_location_invalid_suggestion(database, location): +def get_location_invalid_suggestion(database, location) -> str | None: """Return suggestion what to do when specified location is not valid It gives suggestion when: * A mapset was specified instead of a location. * A GRASS database was specified instead of a location. """ - location_path = os.path.join(database, location) + location_path = Path(database, location) # a common error is to use mapset instead of location, # if that's the case, include that info into the message if is_mapset_valid(location_path): diff --git a/scripts/r.pack/r.pack.py b/scripts/r.pack/r.pack.py index 5029f93c6d2..1f517a427af 100644 --- a/scripts/r.pack/r.pack.py +++ b/scripts/r.pack/r.pack.py @@ -63,8 +63,8 @@ def main(): global tmp tmp = grass.tempdir() - tmp_dir = os.path.join(tmp, infile) - os.mkdir(tmp_dir) + tmp_dir = Path(tmp, infile) + tmp_dir.mkdir(exist_ok=True) grass.debug("tmp_dir = %s" % tmp_dir) gfile = grass.find_file(name=infile, element="cell", mapset=mapset) @@ -150,7 +150,7 @@ def main(): os.path.join(f_tmp_dir, element), ) - if not os.listdir(tmp_dir): + if not any(tmp_dir.iterdir()): grass.fatal(_("No raster map components found")) # copy projection info