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.

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

Add concatenation operator #1

Open
tacaswell opened this issue Jul 1, 2015 · 11 comments · Fixed by #20
Open

Add concatenation operator #1

tacaswell opened this issue Jul 1, 2015 · 11 comments · Fixed by #20
Milestone

Comments

@tacaswell
Copy link
Member

Candidate operator is & such that cycler('c', 'rgb') & cycler('c', 'cmyk') == cycler('c', 'rgbcmyk')

If the keys do not match exactly, raises.

@efiring
Copy link
Member

efiring commented Jul 4, 2015

A problem is that "+" would also be a logical choice for concatenation, but it's already taken. Is there any precedent for using "&" for concatenation? Actually, it seems to me that "&" might be a more consistent operator for what you are now calling "+"; you are cycling property "A" and property "B" together. An alternative would be to skip the concatenation operator and use an extend method, in analogy with list().extend(). That's not so good either, because extend works in place and returns None.

Conclusion: use "&" for inner product composition, "+" for concatenation.

@OceanWolf
Copy link
Member

As far as I see we could still "use" the + operator for this. I say "use" lightly here and you will see why using algebra mixed with python code.

  1. Algebra says we can't add unlike terms together, thus with the "+" operator in its current state we have:

    x = cycler(A, [...])
    y = cycler(B, [...])
    cycle(x + y) = A=cycle(x), B=cycle(y)

    the terms remain separate...

  2. However if we have like terms we can add them together:

    x = cycler(A, [...])
    y = cycler(A, [...])
    cycle(x + y) = A=cycle(x+y)

At the moment, trying to do 2. will error saying we have like keys, but actually this makes sense not only algebraically, but pythonically as well. Currently this error follows the logic that we can't have a repeated keyword argument, plt.plot(color=cycle([green]), color=cycle([red])), but it does make sense if we interpret these as a concatenated keyword, so plt.plot(color=cycle[green, red]).

Now back to the beginning when I said to take this lightly, on the one hand it feels like jamming too much into an operator, but actually now the more I think about it, it feels perfectly right...

Note, that here, as with regular algebra, we can add together products AB + BA, e.g. two cycles that have both color and linestyle set, but we cannot concat cycles like AB + BA + A, as the third cycle leaves us with an undetermined kwarg for B.

@efiring
Copy link
Member

efiring commented Jul 5, 2015

Although I see the logic in your suggested double-overloading of "+", it doesn't lend itself to readable code. Cycler is already a bit on the tricky side--better not to push it over the edge. It would make the "+" operator non-associative; and whether it is commutative would depend on the operands.

@tacaswell
Copy link
Member Author

I am also 👎 on double overloading of +.

I think 'inner product' is going to be much more common than concatenation so I would rather use + for the more common operation. Using + also follows numpy's point-wise operations (rather than pythons list +).

I think the analogy I was going for with & was to sets, but I now see that I had my set operators wrong, | would be correct set-analog for concatenation.

On the other hand, using & for inner product and | for concatenation will make cyclers look 'funny' in the code to tip people off that they are a bit magical. Following this, if we could do py3.5+ only I would advocate for using @ for the outer product instead of *, but I digress.

We could also just follow numpy here and have a concat method.

I also have an urge to use << for something, but this probably isn't the right use.

@OceanWolf
Copy link
Member

From an algebraic stand point I don't see it as double-overloading, but I agree it could get a bit confusing, especially when it comes to debug.

I prefer @efiring suggestions of switching the meanings of + and & as at the level these operators operate we work on 1d lists.

@tacaswell
Copy link
Member Author

From a group theory pov, they are very different. There are two different
groups (and i use that word loosely here, i don't remember the axioms and
proper definitions off the top of my head) that we are combining here; a
group defined by the set of keys and a collection of groups (one per set
of keys) based on the list of values to cycle through. The inner product
moves you between elements of the first group (that is the object you get
back from the operation has a different set of keys), concatenation,
slicing, and scalar multiplication move you around in the second group
(same keys, different values) and outer product does both.

My algebra professors just felt a disturbance in the force due to that
abuse of math, but i think i understand it a lot better now.

We are missing some things to make these real groups: an identity element
and "negative" values.

On Sun, Jul 5, 2015, 2:43 PM OceanWolf notifications@github.com wrote:

From an algebraic stand point I don't see it as double-overloading, but I
agree it could get a bit confusing, especially when it comes to debug.

I prefer @efiring https://github.com/efiring suggestions of switching
the meanings of + and & as at the level these operators operate we work
on 1d lists.


Reply to this email directly or view it on GitHub
#1 (comment).

@OceanWolf
Copy link
Member

Interesting, for an identity, does cycler(None, [None]) count or just the normal identity, as you can multiply by an integer, right? Which reminds me, we also double-overload that operator as well...

Another little issue I have with the inner product, that we must use equal length... as I wrote above, I expect it to return two cycles zipped together, but that shouldn't prevent us from joining cycles of unequal lengths, right?
so

c = cycler('lw', [1,2,3]) + cycler('color', ['r', 'g'])  # with + giving inner product
c = c()
for i in range(6):
  print c.next()

would give

{'lw': 1, 'color': 'r'}
{'lw': 2, 'color': 'g'}
{'lw': 3, 'color': 'r'}
{'lw': 1, 'color': 'g'}
{'lw': 2, 'color': 'r'}
{'lw': 3, 'color': 'g'}

@tacaswell
Copy link
Member Author

Thoughts an tagging a v1.0 without this feature?

@pelson
Copy link
Member

pelson commented Sep 15, 2015

How about putting the functionality in a chain-able method?

new_cycler = cycler.concat(another_cycler)

The operator question can then be delayed a bit, without loss of functionality...

(note I wrote this without properly reading the thread, nor the code, so my comment might be entirely useless 😉 )

tacaswell added a commit to tacaswell/cycler that referenced this issue Oct 3, 2015
Adds a top level function `concat` and a `Cycler` method `concat` which
will concatenate two cyclers.  The method can be chained.

closes matplotlib#1
@pelson
Copy link
Member

pelson commented Oct 12, 2015

Technically, I'm not sure #20 closed this issue, as we only added a function and method, not an operator. @tacaswell - did you want to take the discussion further?

@tacaswell
Copy link
Member Author

I grew pessimistic about getting consensus on what infix operator to use so put the gh-foo close magic in my commit messages.

If we accept my initial infix assignments, then we are left with the choice of & or | for infix concat.

I feel pretty strongly that we should not make the operators do, essentially, broadcasting and guess from looking at the keys if it should zip or concatenate.


https://docs.python.org/3.5/reference/datamodel.html#emulating-numeric-types

infix comment
+ taken
- reserved for inverse of +
* taken
@ 3.5 only
/ reserved for inverse of outer product
// reserved for inverse of something
% reserved for inverse of something
** should be outer if anything
<<, >> confusing semantics
& maybe for concat, might have made more sense for inner
^ should be outer if anything
` `

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants