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
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
8 changes: 6 additions & 2 deletions cmdx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2674,12 +2674,16 @@ def __str__(self):
return str(self.read())

def __repr__(self):
cls_name = '{}.{}'.format(__name__, self.__class__.__name__)
if self._mplug.attribute().apiType() == om.MFn.kCompoundAttribute:
Copy link
Owner

@mottosso mottosso Feb 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would go the opposite route; include rather than exclude. There are lots of types, and if we aren't going to test them all then we should only include the ones we know work. I expect printing meshes and MObject types to also be problematic. Only the float, double, bool and string are interesting, possibly enum too.

Copy link
Contributor Author

@chelloiaco chelloiaco Feb 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of adding more checks into the repr, I thought, since self.typeClass() is doing all the heavy lifting, I should use that info instead.

I had to add a couple more types since a lot of these are types of some basic queries like node['transform'] or node['rotateX'] but this is what I came up with:

...
valid_types = (Angle, Boolean, Distance, Double, Double3, Enum, Float,
               Integer, Long, Matrix, String)
if self.typeClass() not in valid_types:
    return '{}("{}", "{}")'.format(cls_name,
                                    self.node().name(),
                                    self.name())
...

Which worked, with the exception of when the type has not been implemented, since self.typeClass() raises an error. This happens specifically with the following doctest on line 4180:

>>> _ = cmds.file(new=True, force=True)
>>> a = createNode("transform", name="A")
>>> b = createNode("multDoubleLinear", name="B")
>>> a["ihi"] << b["ihi"]
>>> a["ihi"].connection() == b
True
>>> b["ihi"].connection() == a
True
>>> a["ihi"]  # Error here

That is because isHistoricallyInteresting is a kNumericAttribute of kByte NumericData, which isn't supported in self.typeClass()

I could add a try except in the repr like so and then not read() the value:

...
typ_class = None
try:
    typ_class = self.typeClass()
except TypeError:
    pass

if typ_class not in valid_types:
...

But then the doctest above fails for a different reason:

# expected
cmdx.Plug("A", "isHistoricallyInteresting") == 2

# actual repr return
cmdx.Plug("A", "isHistoricallyInteresting")

Would it be ok to remove the value return from that test? Also, in regards to having a try except in the __repr__, I'm not so sure about that idea either, but inevitably there will be cases where the attribute isn’t implemented in typeClass, so it has to account for that.

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, true.. Choices choices.

In that case, 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 ==. It's a __repr__ after all, for debugging. Not performance sensitive or in need to be exact. Then we could potentially work, separately, on making more values print nicely via str(plug)

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)

cls_name = '{}.{}'.format(__name__, self.__class__.__name__)
return '{}("{}", "{}") == {}'.format(cls_name,
self.node().name(),
self.name(),
Expand Down Expand Up @@ -7818,7 +7822,7 @@ def __hash__(self):
def __repr__(self):
"""Avoid repr depicting the full contents of this dict"""
cls_name = '{}.{}'.format(__name__, self.__class__.__name__)
return "<{} : '{}'>".format(cls_name, self["name"])
return 'cmdx.{}("{}")'.format(cls_name, self["name"])

def __new__(cls, *args, **kwargs):
"""Support for using name of assignment
Expand Down
Loading