Skip to content

[High] set_rotation() then add_rotation() raises AttributeError due to inconsistent rotations type #3

Description

@zhaozhiwen

GVolume.set_rotation() reassigns self.rotations to a plain string, but add_rotation() calls self.rotations.append(...). Calling add_rotation after set_rotation raises AttributeError: 'str' object has no attribute 'append'.

Affected files

  • src/pygemc/api/gvolume.py:101 (__init__ initializes self.rotations as a list)
  • src/pygemc/api/gvolume.py:121-142 (set_rotation, add_rotation)

Details

__init__ starts with a list:

self.rotations = ['0*deg, 0*deg, 0*deg']

set_rotation overwrites it with a string:

def set_rotation(self, x, y, z, lunit='deg', order=''):
	with_units = [f"{val}*{lunit}" for val in [x, y, z]]
	string_with_units = ", ".join(with_units)
	if order:
		self.rotations = f"ordered: {order}, {string_with_units}"
	else:
		self.rotations = string_with_units

add_rotation assumes a list:

def add_rotation(self, x, y, z, lunit='deg'):
	...
	self.rotations.append(' + ' + myrotation)

So set_rotation(...) followed by add_rotation(...) crashes. The type of self.rotations is inconsistent (list after construction, str after set_rotation), which also makes get_rotation_string (which iterates) behave differently depending on which setter was last used.

Impact

A natural call sequence (set a base rotation, then add another) crashes. The inconsistent type is fragile and the root cause of related rotation bugs.

Proposed fix

Keep self.rotations a list everywhere. Have set_rotation store a single-element list (replacing any prior rotations, which matches its "define a single, ordered rotation" docstring).

 	def set_rotation(self, x, y, z, lunit='deg', order=''):
 		with_units = [
 			f"{val}*{lunit}"
 			for val in [x, y, z]
 		]
 		string_with_units = ", ".join(with_units)
 		if order:
-			self.rotations = f"ordered: {order}, {string_with_units}"
+			self.rotations = [f"ordered: {order}, {string_with_units}"]
 		else:
-			self.rotations = string_with_units
+			self.rotations = [string_with_units]

With self.rotations always a list, add_rotation's .append(...) works and get_rotation_string's iteration is consistent. (Pair this with the add_rotation/get_rotation_string fix that stops emitting the silently-dropped + form.)


Split out from #102 (one issue per bug). Found via an AI-assisted code review with manual verification against commit 5f8ce875.


Migrated from gemc/src#118 (code lives in this repo, pulled into gemc/src as a Meson subproject).

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type
No fields configured for issues without a type.

Projects

Status
Milestone Planned

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions