cgt icon indicating copy to clipboard operation
cgt copied to clipboard

to support variable length argument in cgt.dimshuffle

Open avostryakov opened this issue 9 years ago • 3 comments

Theano support: tensor.dimshuffle('x', 0, ...) together with tensor.dimshuffle(['x', 0, ...]). I think it is a good idea to support both syntacsis in cgt because theano-people use both.

avostryakov avatar Sep 15 '15 14:09 avostryakov

We will add this functionality in the next little while.

If you can't wait, dimshuffle is entirely contained in cgt.api. Adding this feature would amount to adding a check for a tuple and converting it to a list at the beginning of the function.

bstadie avatar Sep 15 '15 16:09 bstadie

Adding this feature would amount to adding a check for a tuple and converting it to a list at the beginning of the function.

Not quite. It would need changing dimshuffle in core.py from:

    def dimshuffle(self, pattern):
        "see cgt.dimshuffle"
        return cgt.dimshuffle(self, pattern)

to

    def dimshuffle(self, *pattern):
        "see cgt.dimshuffle"
        if len(pattern) == 1 and isinstance(pattern[0], list):
            pattern = pattern[0]
        return cgt.dimshuffle(self, pattern)

Alternatively, you could have Node.dimshuffle pass on *pattern unchanged and modify cgt.dimshuffle to take an argument list and check whether it's a single argument of type list as above. Note that you won't be able to add any positional arguments to dimshuffle in future if you go that route.

f0k avatar Sep 15 '15 17:09 f0k

Ah right. Forgot about modifying the shortcut in core.py. In any case this is a trivial thing to implement.

bstadie avatar Sep 15 '15 18:09 bstadie