Support kw=Collection(task) in addition to Collection('kw', task)

[tuukkamustonen]

Pre-0.23 supported:

ns = Collection(
    env=Collection(some_task1, some_task2),
)

But now it fails (on py3.5 at least) to:

Traceback (most recent call last):
  File "/home/musttu/.pyenv/versions/his/bin/inv", line 11, in <module>
    sys.exit(program.run())
  File "/home/musttu/.pyenv/versions/3.5.5/envs/his/lib/python3.5/site-packages/invoke/program.py", line 325, in run
    self.parse_cleanup()
  File "/home/musttu/.pyenv/versions/3.5.5/envs/his/lib/python3.5/site-packages/invoke/program.py", line 450, in parse_cleanup
    self.list_tasks()
  File "/home/musttu/.pyenv/versions/3.5.5/envs/his/lib/python3.5/site-packages/invoke/program.py", line 665, in list_tasks
    getattr(self, "list_{}".format(self.list_format))()
  File "/home/musttu/.pyenv/versions/3.5.5/envs/his/lib/python3.5/site-packages/invoke/program.py", line 668, in list_flat
    pairs = self._make_pairs(self.scoped_collection, ancestors=[])
  File "/home/musttu/.pyenv/versions/3.5.5/envs/his/lib/python3.5/site-packages/invoke/program.py", line 739, in _make_pairs
    recursed_pairs = self._make_pairs(subcoll, ancestors + [coll])
  File "/home/musttu/.pyenv/versions/3.5.5/envs/his/lib/python3.5/site-packages/invoke/program.py", line 685, in _make_pairs
    ancestor_path = '.'.join(x.name for x in display_ancestors)
TypeError: sequence item 0: expected str instance, NoneType found

Now we have to give the namespace as str parameter to Collection as in:

ns = Collection(
    Collection('env', some_task1, some_task2),
)

Was this change intended or accidental? I liked to old way more (rather kwarg than positional string arg).

[bitprophet]

That sounds accidental to me. Based on the traceback I'm betting it has to do with the overhaul for --list; the collection itself may be instantiating okay and it's something more subtle causing the bug.

[bitprophet]

Suspect this is missed in our test suite because the fixtures I use for testing --list (again, esp after the overhaul) all rely on the pattern where subcollections typically get their name from the module they're part of.

I can confirm we have a test about the actual kwarg-key-is-the-name behavior and it's passing, so this is either limited to --list only, or it's again more subtle.

[bitprophet]

Cool, when I tweak that test fixture I'm getting a big bunch of errors all resembling yours or with same presumed root (a None instead of a string). Thanks for the report...patch should be in soon.

[bitprophet]

Think this is due to an annoying modeling discrepancy: the fact that a task or collection can be added to a Collection multiple times, and thus there are two ways to "name" these objects - the dictionary key they are attached as at a given point, and/or their own internal/intrinsic name.

Most of the time, namespace-crawling functionality is concerned only with the former: something is requesting lookup of eg env.some_task, the root ns goes "ok who in me is env, oh it's this fella", then env is asked for some_task and goes "ah yes I have that key, here is the task object".

This is why, for example, in @tuukkamustonen's case, actually invoking env.some_task is working fine (at least, it is in my test cases).

However, the new list functionality at one point looks at the latter name - the object's intrinsic name - and in the case under examination, it's None (because, as Tuukka stumbled upon, that's driven entirely by whether the inner Collection was given a name argument or first posarg).

---

The naive fix is to modify the object when it's being given as a kwarg value - but this is misleading because as noted above, that's a big no-no (mutating an object you were given by your caller) in general. And in specifics, it would be problematic for us if the object were to be attached elsewhere in the tree later (the name would just get overwritten a second time).

So I'm 99% sure the right fix is to not reference .name in --list but to get it from the context somehow. Hopefully this is feasible...will check.

[bitprophet]

Have that mostly solved but there is another minor add-on problem (I think more an issue with my test than reality), the now-not-a-module subcollection I tweaked lost its docstring, because...I'm handing in the raw collection and not the module.

Technically I should fix this by updating my test output assertions to not expect help text for that particular subcollection, but it makes me realize that there is no way to specify the help for a collection this way - the help functionality has been written assuming one is using module trees.

I'll probably hack it up now to save myself some work, but it means there needs to be a spinoff feature ticket.

[bitprophet]

This is fixed now on master. Doing some unrelated release oriented tasks now but I'll push to GH soon. PyPI release should be within a day or two, hopefully.