Makes listRelatives behave more like Maya cmds

[chelloiaco]

Description of Changes

Potential solution for #20

I changed up the order of the if statements so that the cmdx behaves like the cmds. Some of the behavior is not necessarily to my liking, but it is how cmds work. You will see what I mean in the testing section below. Perhaps a happy medium between both versions would be the best option? Depends on preference I guess.

Another small change I did from the previous behavior was to have the parent return value be an empty list if None, instead of a list with a None: [None].

Testing Done

Using the following setup:

import cmdx
from maya import cmds
from itertools import product

cmds.file(new=True, force=True)

cmds.createNode('transform', name='topGrp')
cmds.polyCube(name='hierarchyCube', constructionHistory=False)
cmds.parent('hierarchyCube', 'topGrp')
cmds.createNode('transform', name='botGrp', parent='hierarchyCube')
cmds.polyCube(name='worldCube', constructionHistory=False)

I ran the following, to test all possible kwargs combination with both versions and have it raise a RuntimeError if the result wasn't the same.

nodes = ('topGrp', 'worldCube')
args = ('children', 'allDescendents', 'parent', 'shapes')
vals = (True, False)
vals_product = product(vals, vals, vals, vals)

for node, vals_tuple in product(nodes, vals_product):
    val_a, val_b, val_c, val_d = vals_tuple
    kwargs = {args[0]: val_a, args[1]: val_b, args[2]: val_c, args[3]: val_d}
    cmds_result = cmds.listRelatives(node, **kwargs) or []
    cmdx_result = [str(i).split('|')[-1] for i in cmdx.listRelatives(node, **kwargs)]
    # Order is different, maybe something to look into in the future
    if set(cmds_result) != set(cmdx_result):
        print(kwargs)
        raise RuntimeError(f'cmds: {cmds_result}, cmdx: {cmdx_result}')

Which raised no RuntimeErrors

Test notes

• I extracted the node name from its fullPath string using split, as a way to keep the current __repr__ behavior of the Nodes and not have that affect the testing. Also, as stated in the code snippet, the returning lists have different orders form the cmds, which I also chose to ignore for the testing.
• Some of the cmds results are a bit misleading, but I aimed to match it regardless, for consistency sake.
  Examples:

  cmds.listRelatives('topGrp', children=False)
  # Result: ['hierarchyCube'] # 
  cmdx.listRelatives('topGrp', children=False)
  # Result: [|topGrp|hierarchyCube] #

  cmds.listRelatives('worldCube', shapes=False)
  # Result: ['worldCubeShape'] # 
  cmdx.listRelatives('worldCube', shapes=False)
  # Result: [|worldCube|worldCubeShape] #

• It turns out the children kwarg is not used with this setup. But, I left it there for now since removing it seemed like a bad idea.

[mottosso]

Very cool, thanks for sharing this.

For your test, can we put this in here? It would make it run whenever anyone makes a new commit, and ensure it remains working from now on.
https://github.com/mottosso/cmdx/blob/master/tests.py

And ideally, in an effort to make it less cryptic and more editable in the future, could you make separate tests for each keyword argument? E..g

def test_listrelatives_shapes():
   # ...

def test_listrelatives_parent():
   # ...

That way, it'll be clear which of the arguments differ and how to add tests for new arguments later.

As for getting their name via split and repr, could we not get the name via obj.name()?

[chelloiaco]

For your test, can we put this in here? It would make it run whenever anyone makes a new commit, and ensure it remains working from now on.
https://github.com/mottosso/cmdx/blob/master/tests.py
And ideally, in an effort to make it less cryptic and more editable in the future, could you make separate tests for each keyword argument?

On it! In fact, this made me end up writing tests that are more robust. And, unfortunately, I found some cases where the output is not the same between both modules (indicated below). So some bugs here for sure! I'll see what needs changing keep updating this PR as needed.

As for getting their name via split and repr, could we not get the name via obj.name()?

We absolutely can. and I am embarrassed that didn't cross my mind 😅
I am doing that for the tests now, thank you!

---

For context, the different outputs happened when testing all created nodes from the following setup:

cmds.createNode('transform', name='topGrp')
cmds.polyCube(name='hierarchyCube', constructionHistory=False)
cmds.parent('hierarchyCube', 'topGrp')
cmds.createNode('transform', name='botGrp', parent='hierarchyCube')
cmds.polyCube(name='worldCube', constructionHistory=False)

And using combination of listRelatives args, which returned these discrepancies:

args = ('hierarchyCube', shapes=True, children=True, allDescendents=False, parent=True)
cmds = ['topGrp']
cmdx = []

args = ('hierarchyCube', shapes=True, children=False, allDescendents=False, parent=True)
cmds = ['topGrp']
cmdx = []

args = ('hierarchyCube', shapes=True, children=False, allDescendents=True, parent=True)
cmds = ['topGrp']
cmdx = []

args = ('hierarchyCube', shapes=True, children=True, allDescendents=True, parent=True)
cmds = ['topGrp']
cmdx = []

args = ('botGrp', shapes=True, children=False, allDescendents=True, parent=True)
cmds = ['hierarchyCube']
cmdx = []

args = ('botGrp', shapes=True, children=True, allDescendents=True, parent=True)
cmds = ['hierarchyCube']
cmdx = []

args = ('botGrp', shapes=True, children=False, allDescendents=False, parent=True)
cmds = ['hierarchyCube']
cmdx = []

args = ('botGrp', shapes=True, children=True, allDescendents=False, parent=True)
cmds = ['hierarchyCube']
cmdx = []

[chelloiaco]

Ok, I'm trying to think of a way to separate the tests but I can't seem to be able to come up with a good solution that isn't too lengthy and not foolproof.

Turns out a lot of the outputs of cmds.listRelatives really depend on what specific combination of kwargs are provided, as some cancel others out.

So, I thought of perhaps making a test "builder" that yields other tests based on the potential combinations given a provided listrelatives_kwargs list (which is currently ['children', 'allDescendents', 'parent', 'shapes']). It looks like the following:

def test_listrelatives():
    """"""

    def check_listrelatives(objects, **kwargs):
        for object in objects:
            cmdx_out = set(cmdx.listRelatives(object, **kwargs) or [])
            cmdx_out = {n.name() for n in cmdx_out}
            cmds_out = set(cmds.listRelatives(object, **kwargs) or [])
            msg = "object='{}', kwargs={}".format(object, kwargs)

            assert_equals(cmdx_out, cmds_out, msg=msg)

    from itertools import product

    # Setup
    new_scene()
    cmds.createNode('transform', name='topGrp')
    cmds.polyCube(name='hierarchyCube', constructionHistory=False)
    cmds.parent('hierarchyCube', 'topGrp')
    cmds.createNode('transform', name='botGrp', parent='hierarchyCube')
    cmds.polyCube(name='worldCube', constructionHistory=False)

    objects = ['topGrp', 'hierarchyCube', 'botGrp', 'worldCube']
    listrelatives_kwargs = [
        'children',
        'allDescendents',
        'parent',
        'shapes'
    ]
    bool_vals_tuples = [(True, False)] * len(listrelatives_kwargs)

    for bool_vals in product(*bool_vals_tuples):
        kws = dict(zip(listrelatives_kwargs, bool_vals))
        msg = (
            "cmdx.listRelatives(" +
            ', '.join(['{}={}'.format(k, v) for k, v in kws.items()]) + ')'
        )
        test_listrelatives.__doc__ = msg
        yield lambda: check_listrelatives(objects, **kws)

Which, while producing a pretty informative output, it will... exponentially pollute the log if more kwargs are added: adding another argument would produce double the amount of output.

cmdx.listRelatives(children=True, allDescendents=True, parent=True, shapes=True) ... ok
cmdx.listRelatives(children=True, allDescendents=True, parent=True, shapes=False) ... ok
cmdx.listRelatives(children=True, allDescendents=True, parent=False, shapes=True) ... ok
cmdx.listRelatives(children=True, allDescendents=True, parent=False, shapes=False) ... ok
cmdx.listRelatives(children=True, allDescendents=False, parent=True, shapes=True) ... ok
cmdx.listRelatives(children=True, allDescendents=False, parent=True, shapes=False) ... ok
cmdx.listRelatives(children=True, allDescendents=False, parent=False, shapes=True) ... ok
cmdx.listRelatives(children=True, allDescendents=False, parent=False, shapes=False) ... ok
cmdx.listRelatives(children=False, allDescendents=True, parent=True, shapes=True) ... ok
cmdx.listRelatives(children=False, allDescendents=True, parent=True, shapes=False) ... ok
cmdx.listRelatives(children=False, allDescendents=True, parent=False, shapes=True) ... ok
cmdx.listRelatives(children=False, allDescendents=True, parent=False, shapes=False) ... ok
cmdx.listRelatives(children=False, allDescendents=False, parent=True, shapes=True) ... ok
cmdx.listRelatives(children=False, allDescendents=False, parent=True, shapes=False) ... ok
cmdx.listRelatives(children=False, allDescendents=False, parent=False, shapes=True) ... ok
cmdx.listRelatives(children=False, allDescendents=False, parent=False, shapes=False) ... ok

And in the case of a fail...

cmdx.listRelatives(children=False, allDescendents=False, parent=False, shapes=False) ... FAIL

The log would look something like:

======================================================================
FAIL: cmdx.listRelatives(children=False, allDescendents=False, parent=False, shapes=False)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\marce\AppData\Roaming\Python\Python37\site-packages\nose\case.py", line 198, in runTest     
    self.test(*self.arg)
  File "D:\repos\cmdx\tests.py", line 626, in check_listrelatives
    assert_equals(cmdx_out, cmds_out, msg=msg)
AssertionError: Items in the second set but not the first:
'hierarchyCube' : object='topGrp', kwargs={'children': False, 'allDescendents': False, 'parent': False, 'shapes': False}

---

A potential middle ground would be to have the function test all the cases silently while keeping track of any non-satisfactory result. After all cases were tested, assert that there were no bad results, logging what went wrong otherwise (formatted like the fail above). In pseudo-code it would be something like:

def test_listrelatives():
    produce all possible kwargs combinations
    initialize fails list
    for each kwarg combination in all possible kwargs combinations:
        if cmdx outputs != cmds outputs:
            append kwargs combination to fails list

     assert fails list is empty, otherwise "formatted_message with each failed comparison"

I could do more literal tests for each children, allDescendents, etc.. kwargs, which would probably be more readable in code. But, as you can probably tell, there would have to be a lot of tests and some would even overlap.

Let me know your thoughts and/or suggestions.

[mottosso]

Hmm, starting to question whether we should adopt the confusing nature of Maya's command. Maya generally stuffs too much logic into its arguments, rather than make another command for it.

That said, the test is too cryptic.

    bool_vals_tuples = [(True, False)] * len(listrelatives_kwargs)

    for bool_vals in product(*bool_vals_tuples):
        kws = dict(zip(listrelatives_kwargs, bool_vals))
        msg = (
            "cmdx.listRelatives(" +
            ', '.join(['{}={}'.format(k, v) for k, v in kws.items()]) + ')'
        )
        test_listrelatives.__doc__ = msg
        yield lambda: check_listrelatives(objects, **kws)

I generally advise against codifying code, and this is a good example why. I have no idea what's going on here. 😅 How about we keep it simple, and test against not every possible permutation of argument, but each argument individually?

def test_listrelatives_shapes():
   _setup_listrelatives_test()

   result_cmds = cmds.listRelatives("pCube", shapes=True)
   result_cmdx = cmdx.listRelatives("pCube", shapes=True)
   assert_equal(result_cmds, result_cmds)

Now anyone can read it, when it fails it's clear why and we/anyone can add another test case. There are 5 arguments, so we'll have 5 tests, for starters. It's true that some combination of flags interfere with each other, but I'd argue that unless there is a usecase demonstrating where and why this is a problem it isn't worth testing or complication tests/implementation for.

[chelloiaco]

Hmm, starting to question whether we should adopt the confusing nature of Maya's command. Maya generally stuffs too much logic into its arguments, rather than make another command for it.

I guess using contradictory kwargs such as children and parent together is kind of a bad idea to begin with... so perhaps it is in fact a bad goal to match to cmds 100%.

I generally advise against codifying code, and this is a good example why. I have no idea what's going on here. 😅

I totally get it. It's nothing really complex, but I understand that it's not readable at a first glance.

How about we keep it simple, and test against not every possible permutation of argument, but each argument individually?

Sounds like a plan. I'll get those tests going and do a commit to this PR when they are done.

[mottosso]

Amazing, these tests are such a relief to read, thanks for that. This looks good, merged!