Skip to content

Commit 5db4e15

Browse files
nimrod-gileadicopybara-github
nimrod-gileadi
authored and
copybara-github
committed
Output attr="false" from PyMJCF when False is explicitly specified.
Before this change, setting any keyword attribute to False meant the attribute wasn't specified at all. This is fine for attributes where the default is false, but wrong for other keyword attributes. In particular, since commit google-deepmind/mujoco@4bfc2c0, ctrllimited and similar attributes changed their default value from "false" to "auto", breaking some models that specify forcelimited=False and forcerange=something. Also in this commit: update the ctrllimited and similar attributes' schemas to include the value "auto". PiperOrigin-RevId: 468699407 Change-Id: I7f64d2e757678a00aec8b1cfa33894ac1b7f5b5b
1 parent 3b801da commit 5db4e15

5 files changed

Lines changed: 71 additions & 51 deletions

File tree

‎dm_control/mjcf/attribute.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ def __init__(self, name, required, parent, value,
184184
conflict_behavior)
185185

186186
def _assign(self, value):
187-
if not value:
187+
if value is None or value == '': # pylint: disable=g-explicit-bool-comparison
188188
self.clear()
189189
else:
190190
try:

‎dm_control/mjcf/attribute_test.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,25 @@ def testKeyword(self):
252252
self.assertXMLStringEqual(mujoco.optional, 'keyword', valid_values[-1])
253253
self.assertCanBeCleared(mujoco.optional, 'keyword')
254254

255+
def testKeywordFalseTrueAuto(self):
256+
mujoco = self._mujoco
257+
for value in ('false', 'False', False):
258+
mujoco.optional.fta = value
259+
self.assertEqual(mujoco.optional.fta, 'false')
260+
self.assertXMLStringEqual(mujoco.optional, 'fta', 'false')
261+
for value in ('true', 'True', True):
262+
mujoco.optional.fta = value
263+
self.assertEqual(mujoco.optional.fta, 'true')
264+
self.assertXMLStringEqual(mujoco.optional, 'fta', 'true')
265+
for value in ('auto', 'AUTO'):
266+
mujoco.optional.fta = value
267+
self.assertEqual(mujoco.optional.fta, 'auto')
268+
self.assertXMLStringEqual(mujoco.optional, 'fta', 'auto')
269+
for value in (None, ''):
270+
mujoco.optional.fta = value
271+
self.assertIsNone(mujoco.optional.fta)
272+
self.assertXMLStringEqual(mujoco.optional, 'fta', None)
273+
255274
def testIdentifier(self):
256275
mujoco = self._mujoco
257276

0 commit comments

Comments
 (0)