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.

Sign up for GitHub

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

Implements a different approach for __repr__ #78

Merged
merged 8 commits into from
Feb 27, 2024
+127 −14
Merged

Implements a different approach for __repr__ #78

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4f335ed
Implements a different approach for __repr__
chelloiaco Feb 17, 2024
5d8a033
Fixes Python2.7 compatibility for doctests
chelloiaco Feb 17, 2024
eb98987
Reimplements old doctests without .name(), .path() and .read()
chelloiaco Feb 18, 2024
b482bef
Changes the repr diplay to be more code-like
chelloiaco Feb 21, 2024
019abe4
Skips read() call within Plug __repr__ if the plug's apiType is of ty…
chelloiaco Feb 22, 2024
73f1aef
Delegates Plug __repr__ value reading to __str__
chelloiaco Feb 23, 2024
8da17d3
Removes try/excepts and implements test case for string plugs
chelloiaco Feb 27, 2024
70857de
Fixes if statement for not isCompound/isArray
chelloiaco Feb 27, 2024
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
33 changes: 20 additions & 13 deletions cmdx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2674,20 +2674,27 @@ def __str__(self):
return str(self.read())

def __repr__(self):
try:
# Delegate the value reading to __str__
read_result = str(self)
Copy link
Owner

Choose a reason for hiding this comment

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

Hmmmm, now str() is a call I would never expect to fail, and if it does then I'd consider that a bug. We can expect it will never fail, and not bother with a valid = True.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PS: I feel like I might've opened a can of worms, but running bs['inputTarget'] causes an API failure when it tries to get 'targetBindMatrix'. I might create a separate issue on it so we keep on topic here.

This is the reason why I added a try/except. running the following causes an API failure:

>>> _ = cmds.file(new=1, f=1)
>>> sphere_a = cmds.polySphere(ch=False)[0]
>>> sphere_b = cmds.polySphere(ch=False)[0]
>>> bs = cmds.blendShape(sphere_a, sphere_b)[0]
>>> bs = cmdx.ls(bs)[0]
>>> bs["inputTarget"].read()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "D:\repos\cmdx\cmdx.py", line 4044, in read

  File "D:\repos\cmdx\cmdx.py", line 5076, in _plug_to_python
    #
  File "D:\repos\cmdx\cmdx.py", line 5094, in _plug_to_python
    )
  File "D:\repos\cmdx\cmdx.py", line 5094, in <genexpr>
    )
  File "D:\repos\cmdx\cmdx.py", line 5076, in _plug_to_python
    #
  File "D:\repos\cmdx\cmdx.py", line 5094, in _plug_to_python
    )
  File "D:\repos\cmdx\cmdx.py", line 5094, in <genexpr>
    )
  File "D:\repos\cmdx\cmdx.py", line 5119, in _plug_to_python
    # E.g. transform["worldMatrix"][0]
RuntimeError: (kFailure): Unexpected Internal Failure

Might be an edge case, but that was the reason why I added tat in.

I did some digging and it could be because the attribute which it fails on (blendshape1.inputTarget[0].inputTargetGroup[0].targetBindMatrix) has its value being cached. It is just a theory at this point though, I haven't properly diagnosed this.

# Isolated API fail:
>>> bs['inputTarget'][0][0][0][4]._mplug.asMObject()             
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: (kFailure): Unexpected Internal Failure

>>> bs['inputTarget'][0][0][0][4]._mplug.isCaching
True

We could remove the try/except and deal with this as a separate issue though, which is probably a cleaner strategy.

valid = True
except:
valid = False

cls_name = '{}.{}'.format(__name__, self.__class__.__name__)
if self._mplug.attribute().apiType() == om.MFn.kCompoundAttribute:
return '{}("{}", "{}")'.format(cls_name,
self.node().name(),
self.name())
read_val = self.read()
if isinstance(read_val, string_types):
# Add surrounding single quotes, indicating the value is a string
read_val = '"{}"'.format(read_val)

return '{}("{}", "{}") == {}'.format(cls_name,
self.node().name(),
self.name(),
read_val)
msg = '{}("{}", "{}")'.format(cls_name,
self.node().name(),
self.name())
if valid:
try:
if self.typeClass() == String:
Copy link
Owner

Choose a reason for hiding this comment

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

Since we've established that typeClass isn't relevant for all types, like kByte, how about we use Maya's apiType here instead? Especially since we're only dealing with a single type.

It would save on the use of try/except too.

Did we also have a test for this case?

Copy link
Contributor Author

@chelloiaco chelloiaco Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since we've established that typeClass isn't relevant for all types, like kByte, how about we use Maya's apiType here instead? Especially since we're only dealing with a single type.

Yeah, I thought of doing it that way as well, but thought it looked less elegant. But sounds good, let me implement that.

Did we also have a test for this case?

We do not, let me add it in.

On another note:

I think we can just put whatever comes out of str(plug) after the ==. It can be a try/except; if it fails, we do not include the ==.

If this is not the case anymore, and we never expect str to fail, would it be ok to expect that it can return None? In the case of a Compound attribute for instance? That way I can add that check to include == or not.

This might be getting a little too much into read() functionality for this PR but, perhaps more useful, instead have it return "Compound" or something of the sort? e.g.:

>>> myNode["myCompoundAttr"]
cmdx.Plug("myNode", myCompoundAttr") == Compound

A third option, albeit more complex, could be to have it return a dictionary, with the child attribute names as keys for instance.

>>> myNode["myCompoundAttr"]
cmdx.Plug("myNode", myCompoundAttr") == {"childA": 1.0, "childB": "Some string value.", "childC": None}

Copy link
Owner

Choose a reason for hiding this comment

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

If this is not the case anymore, and we never expect str to fail, would it be ok to expect that it can return None? In the case of a Compound attribute for instance? That way I can add that check to include == or not.

Sure, let's try it.

A third option, albeit more complex, could be to have it return a dictionary, with the child attribute names as keys for instance.

I mean, this would be supremely useful. But I'm uncertain how stable we can make this.. Maybe let's start small, and add this later on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, let's try it

Scratch that. We can't return something that isn't a string type from __str__. That raises a TypeError, silly me.

I think I was able to think of a decent solution within __repr__, so we're not messing with __str__ in this PR. pushing a commit now which includes this and the above

# Add surrounding quotes, indicating it is a string
read_result = '"{}"'.format(read_result)
except TypeError:
pass
msg += ' == {}'.format(read_result)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to add this in order to avoid cases where it shows a valid value, but of an empty string:

>>> myNode["myStr"]
cmdx.Plug("myNode", "myStr") ==


return msg

def __rshift__(self, other):
"""Support connecting attributes via A >> B"""
Expand Down
Loading