Skip to content
This repository was archived by the owner on Apr 8, 2025. It is now read-only.

Conversation

@sourcery-ai
Copy link

@sourcery-ai sourcery-ai bot commented Dec 4, 2023

Branch master refactored by Sourcery.

If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.

See our documentation here.

Run Sourcery locally

Reduce the feedback loop during development by using the Sourcery editor plugin:

Review changes via command line

To manually merge these changes, make sure you're on the master branch, then run:

git fetch origin sourcery/master
git merge --ff-only FETCH_HEAD
git reset HEAD^

Help us improve this pull request!

@sourcery-ai sourcery-ai bot requested a review from tatstratasys December 4, 2023 15:31
Copy link
Author

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery timed out performing refactorings.

Due to GitHub API limits, only the first 60 comments can be shown.

Comment on lines -79 to -82
try:
with contextlib.suppress(subprocess.CalledProcessError):
return subprocess.check_output("xcode-select -p").strip()
except subprocess.CalledProcessError:
pass
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function GetXcodeDeveloperDirectory refactored with the following changes:

Comment on lines -92 to +94
msvcCompiler = find_executable('cl')
if msvcCompiler:
match = re.search(
"Compiler Version (\d+).(\d+).(\d+)",
subprocess.check_output("cl", stderr=subprocess.STDOUT))
if match:
if msvcCompiler := find_executable('cl'):
if match := re.search(
"Compiler Version (\d+).(\d+).(\d+)",
subprocess.check_output("cl", stderr=subprocess.STDOUT),
):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function GetVisualStudioCompilerAndVersion refactored with the following changes:

Comment on lines -195 to +192
msvcCompilerAndVersion = GetVisualStudioCompilerAndVersion()
if msvcCompilerAndVersion:
if msvcCompilerAndVersion := GetVisualStudioCompilerAndVersion():
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function RunCMake refactored with the following changes:

This removes the following comments ( why? ):

# On MacOS, enable the use of @rpath for relocatable builds.

Comment on lines -237 to +234
PrintInfo("Patching file {filename} (original in {oldFilename})..."
.format(filename=filename, oldFilename=filename + ".old"))
shutil.copy(filename, filename + ".old")
PrintInfo(
"Patching file {filename} (original in {oldFilename})...".format(
filename=filename, oldFilename=f"{filename}.old"
)
)
shutil.copy(filename, f"{filename}.old")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function PatchFile refactored with the following changes:

Comment on lines -253 to +248
filename = url.split("/")[-1]
filename = url.split("/")[-1]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function DownloadURL refactored with the following changes:

if retcode < 0:
return 128 + abs(retcode)
return retcode
return 128 + abs(retcode) if retcode < 0 else retcode
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function _convertRetCode refactored with the following changes:

Comment on lines -162 to +173

if not args.assetName or args.assetName == '':
parser.error("No assetName specified")
stage = CreateModelStage(args.assetName,
assetIdentifier=args.identifier,
kind=args.kind,
filesToReference=args.variantFiles,
variantSetName=args.variantSet,
defaultVariantSelection=args.defaultVariantSelection)

if stage:

if stage := CreateModelStage(
args.assetName,
assetIdentifier=args.identifier,
kind=args.kind,
filesToReference=args.variantFiles,
variantSetName=args.variantSet,
defaultVariantSelection=args.defaultVariantSelection,
):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 162-173 refactored with the following changes:

else:
imagedata = viewportShot

imagedata = windowShot if mailInfo.imagetype == "Window" else viewportShot
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function SendMail refactored with the following changes:

Comment on lines -239 to +292
body = str("Usdview screenshot, taken " + currtime + "\n" +
"----------------------------------------" + "\n" +
"File: "
+ str(usdviewApi.stageIdentifier) + "\n" +
"Selected Prim Paths: "
+ ", ".join(map(str, usdviewApi.selectedPrims)) + "\n" +
"Current Frame: "
+ str(usdviewApi.frame) + "\n" +
"Complexity: "
+ str(usdviewApi.dataModel.viewSettings.complexity) + "\n" +
"Camera Info:\n" +str(camera) + "\n" +
"----------------------------------------" + "\n")
body = str(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
f"Usdview screenshot, taken {currtime}"
+ "\n"
+ "----------------------------------------"
)
+ "\n"
)
+ "File: "
)
+ str(
usdviewApi.stageIdentifier
)
+ "\n"
)
+ "Selected Prim Paths: "
)
+ ", ".join(
map(
str,
usdviewApi.selectedPrims,
)
)
+ "\n"
)
+ "Current Frame: "
)
+ str(usdviewApi.frame)
+ "\n"
)
+ "Complexity: "
)
+ str(usdviewApi.dataModel.viewSettings.complexity)
+ "\n"
)
+ "Camera Info:\n"
)
+ str(camera)
+ "\n"
)
+ "----------------------------------------"
)
+ "\n"
)
)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function _GenerateDefaultInfo refactored with the following changes:

if os.path.exists(outputDir):
if not force:
parser.error('outputDir "%s" exists. Use -f to override' % outputDir)
parser.error(f'outputDir "{outputDir}" exists. Use -f to override')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function main refactored with the following changes:

if os.path.exists(outputDir):
if not force:
parser.error('outputDir "%s" exists. Use -f to override' % outputDir)
parser.error(f'outputDir "{outputDir}" exists. Use -f to override')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function main refactored with the following changes:

with shadingVariant.GetVariantEditContext():
whichBall = variantName.split('_')[-1]
texPath = os.path.join(texDir, 'ball%s.tex' % whichBall)
texPath = os.path.join(texDir, f'ball{whichBall}.tex')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function _AddShadingToBall refactored with the following changes:

# want to deactivate so that we don't delete while iterating.
roomProps = stage.GetPrimAtPath('/World/sets/Room_set/Props')
keepers = set(['Ball_%d' % i for i in [1, 9, 8, 4] ])
keepers = {'Ball_%d' % i for i in [1, 9, 8, 4]}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function _SetupBilliards refactored with the following changes:

Comment on lines -30 to +35
assert([x for x in stage.Traverse()] == [stage.GetPrimAtPath("/refSphere"),
stage.GetPrimAtPath("/refSphere/world"), stage.GetPrimAtPath("/refSphere2"),
stage.GetPrimAtPath("/refSphere2/world")])
assert list(stage.Traverse()) == [
stage.GetPrimAtPath("/refSphere"),
stage.GetPrimAtPath("/refSphere/world"),
stage.GetPrimAtPath("/refSphere2"),
stage.GetPrimAtPath("/refSphere2/world"),
]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 30-63 refactored with the following changes:

return 'h'
else:
return scl[0]
return 'h' if scl == 'GfHalf' else scl[0]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function ScalarSuffix refactored with the following changes:


def err( msg ):
return "ERROR: " + msg + " failed"
return f"ERROR: {msg} failed"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function err refactored with the following changes:

Comment on lines -40 to +42
else:
v = Value()
for i in range(v.dimension):
v[i] = vals[i]
v = Value()
for i in range(v.dimension):
v[i] = vals[i]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function makeValue refactored with the following changes:

Comment on lines -175 to +186
m[0,0] = 1; m[1,0] = 2; m[0,1] = 3; m[1,1] = 4
m[0,0] = 1
m[1,0] = 2
m[0,1] = 3
m[1,1] = 4
self.assertTrue(m[0,0] == 1 and m[1,0] == 2 and m[0,1] == 3 and m[1,1] == 4)

m = Matrix()
m[-1,-1] = 1; m[-2,-1] = 2; m[-1,-2] = 3; m[-2,-2] = 4
m[-1,-1] = 1
m[-2,-1] = 2
m[-1,-2] = 3
m[-2,-2] = 4
self.assertTrue(m[-1,-1] == 1 and m[-2,-1] == 2 and m[-1,-2] == 3 and m[-2,-2] == 4)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestGfMatrix.test_Other refactored with the following changes:

  • Simplify logical expression using De Morgan identities (de-morgan)

Comment on lines -780 to +785
AssertDeterminant(m1 * m1, det1 * det1)
AssertDeterminant(m1 * m1, det1**2)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestGfMatrix.test_Matrix4Determinant refactored with the following changes:


def err( msg ):
return "ERROR: " + msg + " failed"
return f"ERROR: {msg} failed"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function err refactored with the following changes:

Comment on lines -78 to +79
self.assertTrue(not p1 == p2, err("equality"))
self.assertTrue(not p1 != Gf.Plane(Gf.Vec3d(1,1,1), 10), err("inequality"))
self.assertTrue(p1 != p2, err("equality"))
self.assertTrue(p1 == Gf.Plane(Gf.Vec3d(1,1,1), 10), err("inequality"))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestGfBBox3d.test_Operators refactored with the following changes:

  • Simplify logical expression using De Morgan identities [×2] (de-morgan)

Comment on lines -102 to +108
q = q * 10
q *= 10
self.assertEqual(q, Gf.Quaternion(100,Gf.Vec3d(200,300,400)))
q = 10 * q
q *= 10
self.assertEqual(q, Gf.Quaternion(1000,Gf.Vec3d(2000,3000,4000)))
q /= 100
self.assertEqual(q, Gf.Quaternion(10,Gf.Vec3d(20,30,40)))
q = q / 10
q /= 10
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestGfQuaternion.test_Operators refactored with the following changes:

  • Replace assignment with augmented assignment [×3] (aug-assign)

Comment on lines -33 to +35
else:
v = Value()
for i in range(v.dimension):
v[i] = vals[i]
v = Value()
for i in range(v.dimension):
v[i] = vals[i]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function makeValue refactored with the following changes:

self.assertIsInstance(Size(), Size)
self.assertIsInstance(Size(Size()), Size)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestGfSize.runTest refactored with the following changes:

  • Simplify logical expression using De Morgan identities (de-morgan)

elif type == 'i':
return Gf.Vec4i
assert False, "No valid conversion for " + vecType + " to type " + type
assert False, f"No valid conversion for {vecType} to type {type}"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function vecWithType refactored with the following changes:

Comment on lines -148 to +161
self.assertTrue(os.path.isfile("%s/a" % rootDir))
self.assertTrue(os.path.isfile("%s/b" % rootDir))
self.assertTrue(os.path.isfile("%s/c" % rootDir))
self.assertTrue(os.path.isfile(f"{rootDir}/a"))
self.assertTrue(os.path.isfile(f"{rootDir}/b"))
self.assertTrue(os.path.isfile(f"{rootDir}/c"))

self.assertTrue(os.path.isfile("%s/sub1/a" % rootDir))
self.assertTrue(os.path.isfile("%s/sub1/b" % rootDir))
self.assertTrue(os.path.isfile("%s/sub1/c" % rootDir))
self.assertTrue(os.path.isfile(f"{rootDir}/sub1/a"))
self.assertTrue(os.path.isfile(f"{rootDir}/sub1/b"))
self.assertTrue(os.path.isfile(f"{rootDir}/sub1/c"))

self.assertTrue(os.path.isfile("%s/sub2/a" % rootDir))
self.assertTrue(os.path.isfile("%s/sub2/b" % rootDir))
self.assertTrue(os.path.isfile("%s/sub2/c" % rootDir))
self.assertTrue(os.path.isfile(f"{rootDir}/sub2/a"))
self.assertTrue(os.path.isfile(f"{rootDir}/sub2/b"))
self.assertTrue(os.path.isfile(f"{rootDir}/sub2/c"))

self.assertTrue(os.path.isfile("%s/sub3/a" % rootDir))
self.assertTrue(os.path.isfile("%s/sub3/b" % rootDir))
self.assertTrue(os.path.isfile("%s/sub3/c" % rootDir))
self.assertTrue(os.path.isfile(f"{rootDir}/sub3/a"))
self.assertTrue(os.path.isfile(f"{rootDir}/sub3/b"))
self.assertTrue(os.path.isfile(f"{rootDir}/sub3/c"))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestFileUtils.VerifyDirAndFileStructure refactored with the following changes:

Comment on lines -234 to +236
self.log.info('%s %s %s' % (
mtime, Strftime(mtime), self.Links[0][0]))
self.log.info(f'{mtime} {Strftime(mtime)} {self.Links[0][0]}')
for sp,dp in self.Links:
mtime = Lstat(dp)
self.log.info('%s %s %s -> %s' % (
mtime, Strftime(mtime), dp, sp))
self.log.info(f'{mtime} {Strftime(mtime)} {dp} -> {sp}')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestFileUtils.PrintTestLinks refactored with the following changes:


def f4(stringArg):
return 'got string ' + stringArg
return f'got string {stringArg}'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function f4 refactored with the following changes:

Comment on lines -236 to +238
self.assertTrue(1 == Tf._Enum.One)
self.assertTrue(2 == Tf._Enum.Two)
self.assertTrue(3 == Tf._Alpha)
self.assertTrue(Tf._Enum.One == 1)
self.assertTrue(Tf._Enum.Two == 2)
self.assertTrue(Tf._Alpha == 3)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestPython.test_EnumValuesRemovedFromTypeScope refactored with the following changes:

Tf.Debug.SetDebugSymbolsByName('TF_SCRIPT_MODULE_LOADER', True)

prefix = Tf.__package__ + '.testenv.testTfScriptModuleLoader_'
prefix = f'{Tf.__package__}.testenv.testTfScriptModuleLoader_'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 46-46 refactored with the following changes:

Copy link
Author

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Type: Refactoring

Summary of PR: This PR includes changes made by the Sourcery tool to refactor the codebase. It primarily consists of using more concise Python syntax such as the walrus operator for simultaneous assignment and evaluation, generator expressions, and f-strings for string formatting. The changes aim to improve the code's efficiency and readability.

General PR suggestions:

  • Ensure that the use of the walrus operator does not compromise code readability, especially for those who may not be familiar with this newer Python syntax.
  • Verify that changes in logic, particularly where conditions are combined or checks are omitted, do not introduce bugs or alter the intended behavior of the code.
  • Remove unnecessary parentheses that do not contribute to code clarity and are not required by Python syntax.
  • Consider the impact of using set literals on the significance of the order of elements, and use lists or tuples if the order is important.
  • Eliminate semicolons at the end of Python statements, as they are not needed and are not idiomatic in Python.
  • Review the changes to ensure that they align with the project's style guide and maintain the codebase's overall consistency.

Your trial expires on December 18, 2023. Please email tim@sourcery.ai to continue using Sourcery ✨

viewerMode = self.stateProperty("viewerMode", default=False)

if viewerMode:
if viewerMode := self.stateProperty("viewerMode", default=False):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (llm): Using the walrus operator for assignment and condition check is concise, but it may reduce readability for those unfamiliar with the syntax. Consider if readability is a priority over brevity in this context.

Suggested change
if viewerMode := self.stateProperty("viewerMode", default=False):
viewerMode = self.stateProperty("viewerMode", default=False)
if viewerMode:

if self.realStartTimeCode is None or self.realEndTimeCode is None:
self._timeSamples = []

elif self.realStartTimeCode > self.realEndTimeCode:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (llm): The refactoring changes the logic of the time samples update. Previously, the check for 'None' values was separate from the range check, which is now combined. This could lead to a situation where '_timeSamples' is not cleared if only one of 'realStartTimeCode' or 'realEndTimeCode' is 'None'.

Suggested change
elif self.realStartTimeCode > self.realEndTimeCode:
if self.realStartTimeCode is None or self.realEndTimeCode is None or self.realStartTimeCode > self.realEndTimeCode:

# Process some more prim view items.
n = min(100, len(self._itemsToPush))
if n:
if n := min(100, len(self._itemsToPush)):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (llm): While the use of the walrus operator here is compact, it may be less clear that 'n' is intended to be used beyond the scope of the 'if' statement. Consider declaring 'n' before the 'if' statement for clarity.

Suggested change
if n := min(100, len(self._itemsToPush)):
n = min(100, len(self._itemsToPush))
if n:

bottomHeight = 0
stageViewWidth += primViewWidth
primViewWidth = 0
elif self._viewerModeEscapeSizes is None:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (llm): The refactored code changes the behavior when '_viewerModeEscapeSizes' is 'None'. Previously, the 'else' block would always execute if '_viewerModeEscapeSizes' is 'None', but now it only executes if both 'self._viewerMode' and '_viewerModeEscapeSizes' are 'None'. This could lead to unintended behavior.

Suggested change
elif self._viewerModeEscapeSizes is None:
if self._viewerModeEscapeSizes is None or not self._viewerMode:

'https://graphics.pixar.com/usd/docs/api/'
'_usd__page__generating_schemas.html '
'for more information.\n')
errorPrefix = (
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick (llm): The refactoring introduces additional parentheses that are not necessary and could be removed for cleaner code.

assert attr.Get() == 'from_session'
assert (set(stage.GetUsedLayers()) ==
set([sublayer_1, sublayer_2, sessionLayer, rootLayer]))
assert set(stage.GetUsedLayers()) == {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (llm): While using set literals is more performant and readable, ensure that the order of the layers is not significant. If the order matters, a list or tuple might be more appropriate.

title = self._mainWindow.windowTitle()
if title[-1] != '*':
self._mainWindow.setWindowTitle(title + ' *')
self._mainWindow.setWindowTitle(f'{title} *')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise (llm): Good use of f-string for string formatting, which is more readable and efficient.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant