Skip to content

Commit

Permalink
python: Various remaining pyrefact refactorings (#4911)
Browse files Browse the repository at this point in the history
* python: Factor out lines executed in both branches

* style: Use parentheses when assigning tuples

* style: Add parenthesis for clarity

* python: Rewrite negation

* style: Extract constant from loop

* style: Membership test using a set

* style: Factor out commonly used strings into constants

* checks: Remove fixed Ruff exclusions
  • Loading branch information
echoix authored Jan 4, 2025
1 parent 3f98304 commit dd87856
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 107 deletions.
10 changes: 6 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,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 @@ -301,7 +301,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 Down
2 changes: 1 addition & 1 deletion python/grass/gunittest/case.py
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,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
4 changes: 2 additions & 2 deletions python/grass/gunittest/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class _WritelnDecorator:
def __init__(self, stream):
self.stream = stream

def __getattr__(self, attr):
if attr in ("stream", "__getstate__"):
def __getattr__(self, attr: str):
if attr in {"stream", "__getstate__"}:
raise AttributeError(attr)
return getattr(self.stream, attr)

Expand Down
25 changes: 12 additions & 13 deletions python/grass/imaging/images2gif.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def checkImages(images):
# Check and convert dtype
if im.dtype == np.uint8:
images2.append(im) # Ok
elif im.dtype in [np.float32, np.float64]:
elif im.dtype in (np.float32, np.float64):
im = im.copy()
im[im < 0] = 0
im[im > 1] = 1
Expand Down Expand Up @@ -296,7 +296,7 @@ def handleSubRectangles(self, images, subRectangles):
images, xy = self.getSubRectangles(images)

# Done
return images, xy
return (images, xy)

def getSubRectangles(self, ims):
"""Calculate the minimal rectangles that need updating each frame.
Expand All @@ -311,7 +311,7 @@ def getSubRectangles(self, ims):

# Check image count
if len(ims) < 2:
return ims, [(0, 0) for i in ims]
return (ims, [(0, 0) for i in ims])

# We need NumPy
if np is None:
Expand All @@ -334,11 +334,11 @@ def getSubRectangles(self, ims):
Y = np.argwhere(diff.sum(1))
# Get rect coordinates
if X.size and Y.size:
x0, x1 = int(X[0]), int(X[-1] + 1)
y0, y1 = int(Y[0]), int(Y[-1] + 1)
x0, x1 = (int(X[0]), int(X[-1] + 1))
y0, y1 = (int(Y[0]), int(Y[-1] + 1))
else: # No change ... make it minimal
x0, x1 = 0, 2
y0, y1 = 0, 2
x0, x1 = (0, 2)
y0, y1 = (0, 2)

# Cut out and store
im2 = im[y0:y1, x0:x1]
Expand All @@ -349,7 +349,7 @@ def getSubRectangles(self, ims):
# Done
# print('%1.2f seconds to determine subrectangles of %i images' %
# (time.time()-t0, len(ims2)))
return ims2, xy
return (ims2, xy)

def convertImagesToPIL(self, images, dither, nq=0):
"""Convert images to Paletted PIL images, which can then be
Expand All @@ -373,7 +373,7 @@ def convertImagesToPIL(self, images, dither, nq=0):
images2.append(im)

# Convert to paletted PIL images
images, images2 = images2, []
images, images2 = (images2, [])
if nq >= 1:
# NeuQuant algorithm
for im in images:
Expand Down Expand Up @@ -404,7 +404,7 @@ def writeGifToFile(self, fp, images, durations, loops, xys, disposes):
"""

# Obtain palette for all images and count each occurrence
palettes, occur = [], []
palettes, occur = ([], [])
for im in images:
if not pillow:
palette = getheader(im)[1]
Expand Down Expand Up @@ -442,21 +442,20 @@ def writeGifToFile(self, fp, images, durations, loops, xys, disposes):
# Write palette and image data
# Gather info
data = getdata(im)
imdes, data = data[0], data[1:]
imdes, data = (data[0], data[1:])
graphext = self.getGraphicsControlExt(durations[frames], disposes[frames])
# Make image descriptor suitable for using 256 local color palette
lid = self.getImageDescriptor(im, xys[frames])

# Write local header
fp.write(graphext)
if (palette != globalPalette) or (disposes[frames] != 2):
# Use local color palette
fp.write(graphext)
fp.write(lid) # write suitable image descriptor
fp.write(palette) # write local color table
fp.write("\x08") # LZW minimum size code
else:
# Use global color palette
fp.write(graphext)
fp.write(imdes) # write suitable image descriptor

# Write image data
Expand Down
4 changes: 2 additions & 2 deletions python/grass/jupyter/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ def switch_mapset(self, path, location=None, mapset=None):
gisenv = gs.gisenv()
if (
not location
and not mapset
and len(Path(path).parts) == 1
and (not mapset)
and (len(Path(path).parts) == 1)
and mapset_exists(
path=gisenv["GISDBASE"], location=gisenv["LOCATION_NAME"], mapset=path
)
Expand Down
3 changes: 2 additions & 1 deletion python/grass/pygrass/modules/grid/grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,9 @@ def get_works(self):
if self.inlist:
inms = {}
cols = len(box_row)

indx = row * cols + col
for key in self.inlist:
indx = row * cols + col
inms[key] = "%s@%s" % (self.inlist[key][indx], self.mset.name)
# set the computational region, prepare the region parameters
bbox = {k[0]: str(v) for k, v in box.items()[:-2]}
Expand Down
2 changes: 1 addition & 1 deletion python/grass/pygrass/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def is_clean_name(name) -> bool:
False
"""
return not libgis.G_legal_filename(name) < 0
return libgis.G_legal_filename(name) >= 0


def coor2pixel(coord, region):
Expand Down
10 changes: 3 additions & 7 deletions python/grass/pygrass/vector/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,9 @@ def update_odict(self):
"""Read columns name and types from table and update the odict
attribute.
"""
cur = self.conn.cursor()
if self.is_pg():
# is a postgres connection
cur = self.conn.cursor()
cur.execute("SELECT oid,typname FROM pg_type")
diz = dict(cur.fetchall())
odict = OrderedDict()
Expand All @@ -281,17 +281,15 @@ def update_odict(self):
odict[name] = diz[ctype]
except pg.ProgrammingError:
pass
self.odict = odict
else:
# is a sqlite connection
cur = self.conn.cursor()
cur.execute(sql.PRAGMA.format(tname=self.tname))
descr = cur.fetchall()
odict = OrderedDict()
for column in descr:
name, ctype = column[1:3]
odict[name] = ctype
self.odict = odict
self.odict = odict
values = ",".join(
[
"?",
Expand Down Expand Up @@ -363,11 +361,9 @@ def names(self, remove=None, unicod=True):
['cat', 'name', 'value']
"""
nams = list(self.odict.keys())
if remove:
nams = list(self.odict.keys())
nams.remove(remove)
else:
nams = list(self.odict.keys())
if unicod:
return nams
return [str(name) for name in nams]
Expand Down
23 changes: 13 additions & 10 deletions python/grass/script/tests/grass_script_setup_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

import grass.script as gs

RUNTIME_GISBASE_SHOULD_BE_PRESENT = "Runtime (GISBASE) should be present"
SESSION_FILE_NOT_DELETED = "Session file not deleted"

xfail_mp_spawn = pytest.mark.xfail(
multiprocessing.get_start_method() == "spawn",
reason="Multiprocessing using 'spawn' start method requires pickable functions",
Expand Down Expand Up @@ -91,8 +94,8 @@ def test_init_finish_global_functions_capture_strerr0_partial(tmp_path):
)
session_file, runtime_present = run_in_subprocess(init_finish)
assert session_file, "Expected file name from the subprocess"
assert runtime_present, "Runtime (GISBASE) should be present"
assert not os.path.exists(session_file), "Session file not deleted"
assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT
assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED


@xfail_mp_spawn
Expand All @@ -113,8 +116,8 @@ def init_finish(queue):

session_file, runtime_present = run_in_subprocess(init_finish)
assert session_file, "Expected file name from the subprocess"
assert runtime_present, "Runtime (GISBASE) should be present"
assert not os.path.exists(session_file), "Session file not deleted"
assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT
assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED


@xfail_mp_spawn
Expand All @@ -139,8 +142,8 @@ def init_finish(queue):
init_finish
)
assert session_file, "Expected file name from the subprocess"
assert runtime_present, "Runtime (GISBASE) should be present"
assert not os.path.exists(session_file), "Session file not deleted"
assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT
assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED
# This is testing the current implementation behavior, but it is not required
# to be this way in terms of design.
assert runtime_present_after, "Runtime should continue to be present"
Expand Down Expand Up @@ -189,7 +192,7 @@ def init_finish(queue):
) = run_in_subprocess(init_finish)

# Runtime
assert runtime_present_during, "Runtime (GISBASE) should be present"
assert runtime_present_during, RUNTIME_GISBASE_SHOULD_BE_PRESENT
# This is testing the current implementation behavior, but it is not required
# to be this way in terms of design.
assert runtime_present_after, "Expected GISBASE to be present when finished"
Expand All @@ -198,7 +201,7 @@ def init_finish(queue):
assert session_file_present_during, "Expected session file to be present"
assert session_file_variable_present_during, "Variable GISRC should be present"
assert not session_file_variable_present_after, "Not expecting GISRC when finished"
assert not os.path.exists(session_file), "Session file not deleted"
assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED


@xfail_mp_spawn
Expand All @@ -220,8 +223,8 @@ def workload(queue):
session_file, file_existed, runtime_present = run_in_subprocess(workload)
assert session_file, "Expected file name from the subprocess"
assert file_existed, "File should have been present"
assert runtime_present, "Runtime (GISBASE) should be present"
assert not os.path.exists(session_file), "Session file not deleted"
assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT
assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED
assert not os.environ.get("GISRC")
assert not os.environ.get("GISBASE")

Expand Down
Loading

0 comments on commit dd87856

Please sign in to comment.