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: Early exit from functions or loops #4910

Merged
merged 2 commits into from
Jan 16, 2025
Merged
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
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
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
12 changes: 6 additions & 6 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
157 changes: 79 additions & 78 deletions python/grass/gunittest/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,87 +113,88 @@ def discover_modules(
for skip in to_skip:
dirs.remove(skip)

if testsuite_dir in dirs:
dirs.remove(testsuite_dir) # do not recurse to testsuite
full = os.path.join(root, testsuite_dir)

files = os.listdir(full)
if file_pattern:
files = fnmatch.filter(files, file_pattern)
if file_regexp:
files = [f for f in files if re.match(file_regexp, f)]
if exclude:
files = fnmatch_exclude_with_base(files, full, exclude)
files = sorted(files)
# get test/module name without .py
# expecting all files to end with .py
# this will not work for invoking bat files but it works fine
# as long as we handle only Python files (and using Python
# interpreter for invoking)

# TODO: warning about no tests in a testsuite
# (in what way?)
for file_name in files:
# TODO: add also import if requested
# (see older versions of this file)
# TODO: check if there is some main in .py
# otherwise we can have successful test just because
# everything was loaded into Python
# TODO: check if there is set -e or exit !0 or ?
# otherwise we can have successful because nothing was reported
abspath = os.path.abspath(full)
abs_file_path = os.path.join(abspath, file_name)
if file_name.endswith(".py"):
if file_name == "__init__.py":
# we always ignore __init__.py
continue
file_type = "py"
name = file_name[:-3]
elif file_name.endswith(".sh"):
file_type = "sh"
name = file_name[:-3]
if testsuite_dir not in dirs:
continue
dirs.remove(testsuite_dir) # do not recurse to testsuite
full = os.path.join(root, testsuite_dir)

files = os.listdir(full)
if file_pattern:
files = fnmatch.filter(files, file_pattern)
if file_regexp:
files = [f for f in files if re.match(file_regexp, f)]
if exclude:
files = fnmatch_exclude_with_base(files, full, exclude)
files = sorted(files)
# get test/module name without .py
# expecting all files to end with .py
# this will not work for invoking bat files but it works fine
# as long as we handle only Python files (and using Python
# interpreter for invoking)

# TODO: warning about no tests in a testsuite
# (in what way?)
for file_name in files:
# TODO: add also import if requested
# (see older versions of this file)
# TODO: check if there is some main in .py
# otherwise we can have successful test just because
# everything was loaded into Python
# TODO: check if there is set -e or exit !0 or ?
# otherwise we can have successful because nothing was reported
abspath = os.path.abspath(full)
abs_file_path = os.path.join(abspath, file_name)
if file_name.endswith(".py"):
if file_name == "__init__.py":
# we always ignore __init__.py
continue
file_type = "py"
name = file_name[:-3]
elif file_name.endswith(".sh"):
file_type = "sh"
name = file_name[:-3]
else:
file_type = None # alternative would be '', now equivalent
name = file_name

add = False
try:
if grass_location == all_locations_value:
add = True
else:
file_type = None # alternative would be '', now equivalent
name = file_name

add = False
try:
if grass_location == all_locations_value:
add = True
else:
try:
locations = ["nc", "stdmaps", "all"]
except AttributeError:
add = True # test is universal
else:
if universal_location_value in locations:
add = True # cases when it is explicit
if grass_location in locations:
add = True # standard case with given location
if not locations:
add = True # count not specified as universal
except ImportError as e:
if add_failed_imports:
add = True
try:
locations = ["nc", "stdmaps", "all"]
except AttributeError:
add = True # test is universal
else:
raise ImportError(
"Cannot import module named"
" %s in %s (%s)" % (name, full, e.message)
)
# alternative is to create TestClass which will raise
# see unittest.loader
if add:
modules.append(
GrassTestPythonModule(
name=name,
module=None,
tested_dir=root,
file_dir=full,
abs_file_path=abs_file_path,
file_path=os.path.join(full, file_name),
file_type=file_type,
)
if universal_location_value in locations:
add = True # cases when it is explicit
if grass_location in locations:
add = True # standard case with given location
if not locations:
add = True # count not specified as universal
except ImportError as e:
if add_failed_imports:
add = True
else:
raise ImportError(
"Cannot import module named %s in %s (%s)"
% (name, full, e.message)
)
# alternative is to create TestClass which will raise
# see unittest.loader
if add:
modules.append(
GrassTestPythonModule(
name=name,
module=None,
tested_dir=root,
file_dir=full,
abs_file_path=abs_file_path,
file_path=os.path.join(full, file_name),
file_type=file_type,
)
)
# in else with some verbose we could tell about skipped test
return modules

Expand Down
6 changes: 2 additions & 4 deletions python/grass/gunittest/reporters.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,10 +527,8 @@ def success_to_html_text(total, successes):
def success_to_html_percent(total, successes):
if total:
pass_per = 100 * (float(successes) / total)
pass_per = percent_to_html(pass_per)
else:
pass_per = UNKNOWN_NUMBER_HTML
return pass_per
return percent_to_html(pass_per)
return UNKNOWN_NUMBER_HTML


class GrassTestFilesHtmlReporter(GrassTestFilesCountingReporter):
Expand Down
15 changes: 8 additions & 7 deletions python/grass/imaging/images2gif.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,14 @@ def handleSubRectangles(self, images, subRectangles):
# First make numpy arrays if required
for i in range(len(images)):
im = images[i]
if isinstance(im, Image.Image):
tmp = im.convert() # Make without palette
a = np.asarray(tmp)
if len(a.shape) == 0:
msg = "Too little memory to convert PIL image to array"
raise MemoryError(msg)
images[i] = a
if not isinstance(im, Image.Image):
continue
tmp = im.convert() # Make without palette
a = np.asarray(tmp)
if len(a.shape) == 0:
msg = "Too little memory to convert PIL image to array"
raise MemoryError(msg)
images[i] = a

# Determine the sub rectangles
images, xy = self.getSubRectangles(images)
Expand Down
5 changes: 2 additions & 3 deletions python/grass/imaging/images2ims.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,9 @@ def _getSequenceNumber(filename, part1, part2):
# Get all numeric chars
seq2 = ""
for c in seq:
if c in digits:
seq2 += c
else:
if c not in digits:
break
seq2 += c
# Make int and return
return int(seq2)

Expand Down
19 changes: 9 additions & 10 deletions python/grass/imaging/images2swf.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,15 +551,14 @@ def __init__(self, im):
# when storing RGB as ARGB).

if len(im.shape) == 3:
if im.shape[2] in [3, 4]:
tmp = np.ones((im.shape[0], im.shape[1], 4), dtype=np.uint8) * 255
for i in range(3):
tmp[:, :, i + 1] = im[:, :, i]
if im.shape[2] == 4:
tmp[:, :, 0] = im[:, :, 3] # swap channel where alpha is in
else:
if im.shape[2] not in {3, 4}:
msg = "Invalid shape to be an image."
raise ValueError(msg)
tmp = np.ones((im.shape[0], im.shape[1], 4), dtype=np.uint8) * 255
for i in range(3):
tmp[:, :, i + 1] = im[:, :, i]
if im.shape[2] == 4:
tmp[:, :, 0] = im[:, :, 3] # swap channel where alpha is in

elif len(im.shape) == 2:
tmp = np.ones((im.shape[0], im.shape[1], 4), dtype=np.uint8) * 255
Expand Down Expand Up @@ -807,11 +806,11 @@ def writeSwf(filename, images, duration=0.1, repeat=True):

# Check duration
if hasattr(duration, "__len__"):
if len(duration) == len(images2):
duration = list(duration)
else:
if len(duration) != len(images2):
msg = "len(duration) doesn't match amount of images."
raise ValueError(msg)
duration = list(duration)

else:
duration = [duration for im in images2]

Expand Down
9 changes: 4 additions & 5 deletions python/grass/jupyter/map.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,12 @@ def run(self, module, **kwargs):
:param `**kwargs`: named arguments passed to run_command()"""

# Check module is from display library then run
if module[0] == "d":
self._region_manager.set_region_from_command(module, **kwargs)
self._region_manager.adjust_rendering_size_from_region()
gs.run_command(module, env=self._env, **kwargs)
else:
if module[0] != "d":
msg = "Module must begin with letter 'd'."
raise ValueError(msg)
self._region_manager.set_region_from_command(module, **kwargs)
self._region_manager.adjust_rendering_size_from_region()
gs.run_command(module, env=self._env, **kwargs)

def __getattr__(self, name):
"""Parse attribute to GRASS display module. Attribute should be in
Expand Down
12 changes: 5 additions & 7 deletions python/grass/pygrass/gis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,11 @@ def glist(self, type, pattern=None):
elist = []
for el in clist:
el_name = ct.cast(el, ct.c_char_p).value
if el_name:
elist.append(decode(el_name))
else:
if not el_name:
if pattern:
return fnmatch.filter(elist, pattern)
return elist
elist.append(decode(el_name))

def is_current(self):
"""Check if the MAPSET is the working MAPSET"""
Expand Down Expand Up @@ -459,12 +458,11 @@ def add(self, mapset):
:param mapset: a mapset's name
:type mapset: str
"""
if mapset not in self.read() and mapset in self.location:
with open(self.spath, "a+") as f:
f.write("%s\n" % mapset)
else:
if mapset in self.read() or mapset not in self.location:
msg = "Mapset not found"
raise TypeError(msg)
with open(self.spath, "a+") as f:
f.write("%s\n" % mapset)

def remove(self, mapset):
"""Remove mapset to the search path
Expand Down
10 changes: 5 additions & 5 deletions python/grass/pygrass/modules/interface/flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ def get_bash(self):
>>> flag.get_bash()
'--o'
"""
if self.value:
if self.special:
return "--%s" % self.name[0]
return "-%s" % self.name
return ""
if not self.value:
return ""
if self.special:
return "--%s" % self.name[0]
return "-%s" % self.name

def get_python(self):
"""Return the python representation of a flag.
Expand Down
Loading
Loading