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

python: Pyrefact #334

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ ignore = [
"E501", # line-too-long
"E721", # type-comparison
"E722", # bare-except
"E731", # lambda-assignment
"E741", # ambiguous-variable-name
"F403", # undefined-local-with-import-star
"F405", # undefined-local-with-import-star-usage
Expand All @@ -149,7 +148,6 @@ ignore = [
"N803", # invalid-argument-name
"N806", # non-lowercase-variable-in-function
"N812", # lowercase-imported-as-non-lowercase
"N814", # camelcase-imported-as-constant
"N815", # mixed-case-variable-in-class-scope
"N816", # mixed-case-variable-in-global-scope
"N818", # error-suffix-on-exception-name
Expand Down Expand Up @@ -220,7 +218,6 @@ ignore = [
"S108", # hardcoded-temp-file
"S110", # try-except-pass
"S112", # try-except-continue
"S113", # request-without-timeout
"S202", # tarfile-unsafe-members
"S307", # suspicious-eval-usage
"S310", # suspicious-url-open-usage
Expand All @@ -240,12 +237,10 @@ ignore = [
"SIM102", # collapsible-if
"SIM105", # suppressible-exception
"SIM113", # enumerate-for-loop
"SIM116", # if-else-block-instead-of-dict-lookup
"SIM118", # in-dict-keys
"SIM223", # expr-and-false
"SLF001", # private-member-access
"TRY002", # raise-vanilla-class
"TRY003", # raise-vanilla-args
"TRY004", # type-check-without-type-error
"TRY201", # verbose-raise
"TRY300", # try-consider-else
Expand All @@ -262,10 +257,8 @@ ignore = [
# "A005", # builtin-module-shadowing
# "PLW0108", # unnecessary-lambda
# "SIM115", # open-file-with-context-handler
# Ignore `E402` (import violations) in all `__init__.py` files
"**.py" = ["PYI066"]
"*/testsuite/**.py" = ["PT009", "PT027"]
"__init__.py" = ["E402"]
"display/d.mon/render_cmd.py" = ["SIM115"]
"gui/**" = ["PLW0108"] # See https://github.com/OSGeo/grass/issues/4124
"gui/wxpython/animation/temporal_manager.py" = ["SIM115"]
Expand All @@ -283,15 +276,15 @@ ignore = [
"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"]
"gui/wxpython/mapdisp/main.py" = ["SIM115"]
"gui/wxpython/modules/colorrules.py" = ["SIM115"]
"gui/wxpython/modules/mcalc_builder.py" = ["SIM115"]
"gui/wxpython/photo2image/*.py" = ["SIM115"]
"gui/wxpython/psmap/*.py" = ["SIM115"]
"gui/wxpython/photo2image/ip2i_manager.py" = ["SIM115"]
"gui/wxpython/psmap/dialogs.py" = ["PTH208"]
"gui/wxpython/psmap/frame.py" = ["SIM115"]
"gui/wxpython/psmap/instructions.py" = ["SIM115"]
"gui/wxpython/psmap/utils.py" = ["PGH004"]
"gui/wxpython/rdigit/controller.py" = ["SIM115"]
"gui/wxpython/rlisetup/*.py" = ["SIM115"]
Expand All @@ -300,7 +293,9 @@ ignore = [
"gui/wxpython/tools/update_menudata.py" = ["SIM115"]
"gui/wxpython/tplot/frame.py" = ["FLY002"]
"gui/wxpython/vdigit/mapwindow.py" = ["SIM115"]
"gui/wxpython/vnet/*.py" = ["SIM115"]
"gui/wxpython/vnet/vnet_core.py" = ["SIM115"]
"gui/wxpython/vnet/vnet_data.py" = ["SIM115"]
"gui/wxpython/vnet/widgets.py" = ["SIM115"]
"gui/wxpython/web_services/dialogs.py" = ["SIM115"]
"gui/wxpython/wxplot/profile.py" = ["A005", "SIM115"]
"imagery/i.atcorr/create_iwave.py" = ["SIM115"]
Expand All @@ -321,7 +316,6 @@ ignore = [
"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"]
Expand All @@ -340,9 +334,7 @@ ignore = [
"python/grass/pygrass/vector/testsuite/test_table.py" = ["PLW0108"]
"python/grass/script/array.py" = ["A005"]
"python/grass/script/core.py" = ["PTH208"]
"python/grass/script/db.py" = ["SIM115"]
"python/grass/script/raster.py" = ["SIM115"]
"python/grass/script/utils.py" = ["FURB189", "SIM115"]
"python/grass/script/utils.py" = ["FURB189"]
"python/grass/temporal/aggregation.py" = ["SIM115"]
"python/grass/temporal/register.py" = ["SIM115"]
"python/grass/temporal/stds_export.py" = ["SIM115"]
Expand Down Expand Up @@ -385,7 +377,6 @@ ignore = [
"temporal/t.register/testsuite/test_t_register_raster.py" = ["FLY002"]
"temporal/t.register/testsuite/test_t_register_raster_file.py" = ["FLY002"]
"temporal/t.remove/t.remove.py" = ["SIM115"]
"temporal/t.unregister/t.unregister.py" = ["SIM115"]
"utils/**.py" = ["SIM115"]
"utils/generate_release_notes.py" = ["PGH004"]
"utils/thumbnails.py" = ["PTH208"]
Expand Down
4 changes: 1 addition & 3 deletions python/grass/benchmark/results.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@ def join_results_from_files(
source_filenames, prefixes=None, select=None, prefixes_as_labels=False
):
"""Join multiple files into one results object."""
to_merge = []
for result_file in source_filenames:
to_merge.append(load_results_from_file(result_file))
to_merge = [load_results_from_file(result_file) for result_file in source_filenames]
return join_results(
to_merge,
prefixes=prefixes,
Expand Down
25 changes: 11 additions & 14 deletions python/grass/benchmark/testsuite/test_benchmark.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,10 @@ def test_resolutions(self):
},
]
resolutions = [300, 200, 100]
results = []
for benchmark in benchmarks:
results.append(
benchmark_resolutions(
**benchmark,
resolutions=resolutions,
)
)
results = [
benchmark_resolutions(**benchmark, resolutions=resolutions)
for benchmark in benchmarks
]
plot_file = "test_res_plot.png"
num_cells_plot(results, filename=plot_file)
self.assertTrue(Path(plot_file).is_file())
Expand All @@ -76,9 +72,9 @@ def test_single(self):
"label": label,
}
]
results = []
for benchmark in benchmarks:
results.append(benchmark_single(**benchmark, repeat=repeat))
results = [
benchmark_single(**benchmark, repeat=repeat) for benchmark in benchmarks
]
self.assertEqual(len(results), len(benchmarks))
for result in results:
self.assertTrue(hasattr(result, "all_times"))
Expand All @@ -100,9 +96,10 @@ def test_nprocs(self):
"max_nprocs": 4,
}
]
results = []
for benchmark in benchmarks:
results.append(benchmark_nprocs(**benchmark, repeat=repeat, shuffle=True))
results = [
benchmark_nprocs(**benchmark, repeat=repeat, shuffle=True)
for benchmark in benchmarks
]
self.assertEqual(len(results), len(benchmarks))
for result in results:
self.assertTrue(hasattr(result, "times"))
Expand Down
3 changes: 1 addition & 2 deletions python/grass/benchmark/testsuite/test_benchmark_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ class TestBenchmarkCLI(TestCase):
"""Tests that benchmarkin CLI works"""

json_filename = "plot_test.json"
png_filenames = [f"plot_test1_{i}.png" for i in range(4)]
png_filenames.append("plot_test2.png")
png_filenames = [*[f"plot_test1_{i}.png" for i in range(4)], "plot_test2.png"]

def tearDown(self):
"""Remove test files"""
Expand Down
5 changes: 2 additions & 3 deletions python/grass/experimental/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,13 @@ def require_create_ensure_mapset(
)
exists = mapset_exists(path)
if create and exists:
if overwrite:
delete_mapset(path.directory, path.location, path.mapset)
else:
if not overwrite:
msg = (
f"Mapset '{path.mapset}' already exists, "
"use a different name, overwrite, or ensure"
)
raise ValueError(msg)
delete_mapset(path.directory, path.location, path.mapset)
if create or (ensure and not exists):
create_mapset(path.directory, path.location, path.mapset)
elif not exists or not is_mapset_valid(path):
Expand Down
25 changes: 9 additions & 16 deletions python/grass/grassdb/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import grass.grassdb.config as cfg
import grass.script as gs
from grass.script import gisenv
from itertools import starmap


def mapset_exists(path: str | os.PathLike[str], location=None, mapset=None) -> bool:
Expand Down Expand Up @@ -536,10 +537,7 @@ def get_reasons_locations_not_removable(locations):

Returns messages as list if there were any failed checks, otherwise empty list.
"""
messages = []
for grassdb, location in locations:
messages += get_reasons_location_not_removable(grassdb, location)
return messages
return list(starmap(get_reasons_location_not_removable, locations))


def get_reasons_location_not_removable(grassdb, location):
Expand Down Expand Up @@ -570,9 +568,7 @@ def get_reasons_location_not_removable(grassdb, location):
)

# Append to the list of tuples
mapsets = []
for g_mapset in g_mapsets:
mapsets.append((grassdb, location, g_mapset))
mapsets = [(grassdb, location, g_mapset) for g_mapset in g_mapsets]

# Concentenate both checks
messages += get_reasons_mapsets_not_removable(mapsets, check_permanent=False)
Expand Down Expand Up @@ -601,9 +597,7 @@ def get_reasons_grassdb_not_removable(grassdb):
g_locations = get_list_of_locations(grassdb)

# Append to the list of tuples
locations = []
for g_location in g_locations:
locations.append((grassdb, g_location))
locations = [(grassdb, g_location) for g_location in g_locations]
return get_reasons_locations_not_removable(locations)


Expand All @@ -614,12 +608,11 @@ def get_list_of_locations(dbase):

:return: list of locations (sorted)
"""
locations = []
for location in glob.glob(os.path.join(dbase, "*")):
if os.path.join(location, "PERMANENT") in glob.glob(
os.path.join(location, "*")
):
locations.append(os.path.basename(location))
locations = [
os.path.basename(location)
for location in glob.glob(os.path.join(dbase, "*"))
if os.path.join(location, "PERMANENT") in glob.glob(os.path.join(location, "*"))
]

locations.sort(key=lambda x: x.lower())

Expand Down
10 changes: 4 additions & 6 deletions python/grass/grassdb/history.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,10 @@ def add_entry(history_path, entry):
:param str history_path: path to the history log file
:param dict entry: entry consisting of 'command' and 'command_info' keys
"""
if get_history_file_extension(history_path) == ".json":
_add_entry_to_JSON(history_path, entry)
else:
if get_history_file_extension(history_path) != ".json":
msg = "Adding entries is supported only for JSON format."
raise ValueError(msg)
_add_entry_to_JSON(history_path, entry)


def _update_entry_in_JSON(history_path, command_info, index=None):
Expand Down Expand Up @@ -358,11 +357,10 @@ def update_entry(history_path, command_info, index=None):
:param dict command_info: command info entry for update
:param int|None index: index of the command to be updated
"""
if get_history_file_extension(history_path) == ".json":
_update_entry_in_JSON(history_path, command_info, index)
else:
if get_history_file_extension(history_path) != ".json":
msg = "Updating entries is supported only for JSON format."
raise ValueError(msg)
_update_entry_in_JSON(history_path, command_info, index)


def copy(history_path, target_path):
Expand Down
14 changes: 7 additions & 7 deletions python/grass/gunittest/case.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,7 @@ def assertModuleKeyValue(
" provided in reference"
": %s\n" % (module, ", ".join(missing))
)
if mismatch:
stdMsg = "%s difference:\n" % module
stdMsg += "mismatch values"
stdMsg += " (key, reference, actual): %s\n" % mismatch
stdMsg += "command: %s %s" % (module, parameters)
else:
if not mismatch:
# we can probably remove this once we have more tests
# of keyvalue_equals and diff_keyvalue against each other
msg = (
Expand All @@ -273,6 +268,11 @@ def assertModuleKeyValue(
" (assertModuleKeyValue())"
)
raise RuntimeError(msg)
stdMsg = "%s difference:\n" % module
stdMsg += "mismatch values"
stdMsg += " (key, reference, actual): %s\n" % mismatch
stdMsg += "command: %s %s" % (module, parameters)

self.fail(self._formatMessage(msg, stdMsg))

def assertRasterFitsUnivar(self, raster, reference, precision=None, msg=None):
Expand Down Expand Up @@ -1335,7 +1335,7 @@ def runModule(cls, module, expecting_stdout=False, **kwargs):
module.name, module.get_python(), module.returncode, errors=errors
)
# TODO: use this also in assert and apply when appropriate
if expecting_stdout and not module.outputs.stdout.strip():
if expecting_stdout and (not module.outputs.stdout.strip()):
if module.outputs.stderr:
errors = " The errors are:\n" + module.outputs.stderr
else:
Expand Down
Loading
Loading