You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I think if something gets added it should be keyword controlled like whitespace, but also wondering if at that point we're writing a patchwork expression evaluator (since | is vertical seperation and + is horizontal composition and I'm sure will be the next request from some folks even though it's implicit...)
The string layout definition already restricts names to single characters. Therefore, the limitation would only be that you cannot name an Axes | in that case. We don't need to impose a limitation on the list layout names.
The only use case I could come up with, would be layout='|-' but then again, you could also use layout='vh' (for vertical, horizontal). IMHO the possibility for a compact readable notation outweight a rather obscure use case of | as Axes name.
I don't think we should make everything configurable via keywords. This is a domain specific language and we're defining it. There's little point in allowing various dialects.
I don't think we need to make everything controllable via keywords. This a mini-language and we're defining it. There's little point in allowing various dialects (Not even sure we want to keep empty_sentinel in the long run).
As a side note, what we are doing is a bit different from patchwork expressions. Patchwork expressions are operators to compose elements (for the layout (A | B) / C), you need explicit operators because the elements are variables. However we just declare a layout (AB|CC) and have markers for empty and next row. We could use the slash for next row AB/CC because that's the row -compsition operator in patchwork. But IMO its reads a little worse and it's sort of a false friend as it's similar in semantic but a different concept, e.g. we won't have any of the other patchwork operators.
I read the patchworks docs wrong, and now I'm a little concerned that designating '|' as vertical when patchwork designates '|' as horizontal is gonna be very confusing.
I am not super concerned about the dialect collision with patchwork, as @timhoffm says we are developing a mini declarative DSL that is sufficiently different we may not want to provide the false sense of similarity.
I am not super concerned about the dialect collision with patchwork, as @timhoffm says we are developing a mini declarative DSL that is sufficiently different we may not want to provide the false sense of similarity.
In this case though patchwork was a direct motivator and we expect overlapping user base so this seems like a point of confusion that we kind of can anticipate and I don't get walking directly into that conflict, especially since as Anthony points out \n works (and then we don't need to do an extra level of parsing.
My primary concern is: Which specification reads best and is most intuitive for our average user:
plt.subplot_mosaic('AB|CC')
plt.subplot_mosaic('AB/CC')
plt.subplot_mosaic('AB\nCC')
In my opinion, the first one wins by large. The concerns raised above are valid but don't outweigh that advantage.
Re: Added complexity
The amount of extra work compared to the \n-version is negligible - probably one line of code and two lines of docs.
Re: Relation to patchwork
I acknowledge that moasic is inspired by one specific part of patchwork namely the textual layout mechanism. It's only that. We don't have anything to do with patchwork composition operators (also explicitly stated in the tutorial: "While we do not implement the operator overloading style, we do provide a Pythonic API for specifying (nested) Axes layouts." ).
Patchwork does not allow single-line layout strings, so there cannot be a strong expectation how the line separation char should look like. If I'd have to define single-line layout string for patchwork, the row composition operator would be a strong suggestion. However, we have our own layout specification, which is only loosely related to patchwork; e.g. we chose to use a different empty placeholder by default.
Given the association would only be indirect over two steps (mpl layout --> patchwork layout --> patchwork compostion operator), I don't by that patchwork users would be significantly confused. Moreover, only a minority of our users will know patchwork anyway.
Moreover, only a minority of our users will know patchwork anyway.
It'd be the R users who already tend to be (understandably) frustrated that we have a different paradigm. I very much worry about the frustration of '|' means vertical alignment here and 'I' means horizontal alignment in the closest analogue they also might be using concurrently.
What about borrowing from the other layout and using "," so "AA,BB"?
Reading below this line is at own personal risk. Just noted down some crazy ideas so that they don't get lost.
This is not an official API propsal.
At the danger of annoying everybody here again: ; as row separator almost calls for , as (optional) column separtor and allowing multi-char names in strings. At least for simple cases that'd be nice:
plt.subplot_mosaic('top;bottom')
plt.subplot_mosaic('left,right')
plt.subplot_mosaic('left,right;bottom,bottom')
Oh, or alternatively mapping single chars to long names:
Activity
anntzer commentedon Oct 14, 2020
I'm not against
|, but note that you can already write"AB\nCC"which is not that bad...tacaswell commentedon Oct 14, 2020
The vertical bar definitely scans better though, I'm mostly 👍, only concern is adding
|to white space as things that may not be Axes names.story645 commentedon Oct 15, 2020
I think if something gets added it should be keyword controlled like whitespace, but also wondering if at that point we're writing a patchwork expression evaluator (since | is vertical seperation and + is horizontal composition and I'm sure will be the next request from some folks even though it's implicit...)
timhoffm commentedon Oct 15, 2020
The string layout definition already restricts names to single characters. Therefore, the limitation would only be that you cannot name an Axes
|in that case. We don't need to impose a limitation on the list layout names.The only use case I could come up with, would be
layout='|-'but then again, you could also uselayout='vh'(for vertical, horizontal). IMHO the possibility for a compact readable notation outweight a rather obscure use case of|as Axes name.I don't think we should make everything configurable via keywords. This is a domain specific language and we're defining it. There's little point in allowing various dialects.
I don't think we need to make everything controllable via keywords. This a mini-language and we're defining it. There's little point in allowing various dialects (Not even sure we want to keep
empty_sentinelin the long run).As a side note, what we are doing is a bit different from patchwork expressions. Patchwork expressions are operators to compose elements (for the layout
(A | B) / C), you need explicit operators because the elements are variables. However we just declare a layout (AB|CC) and have markers for empty and next row. We could use the slash for next rowAB/CCbecause that's the row -compsition operator in patchwork. But IMO its reads a little worse and it's sort of a false friend as it's similar in semantic but a different concept, e.g. we won't have any of the other patchwork operators.story645 commentedon Oct 15, 2020
I read the patchworks docs wrong, and now I'm a little concerned that designating '|' as vertical when patchwork designates '|' as horizontal is gonna be very confusing.
tacaswell commentedon Oct 15, 2020
I am not super concerned about the dialect collision with patchwork, as @timhoffm says we are developing a mini declarative DSL that is sufficiently different we may not want to provide the false sense of similarity.
jklymak commentedon Oct 15, 2020
I agree with most of @timhoffm points, but I'm mildly against a lot of parsing logic here. There is no need for this to grow more complex.
story645 commentedon Oct 15, 2020
In this case though patchwork was a direct motivator and we expect overlapping user base so this seems like a point of confusion that we kind of can anticipate and I don't get walking directly into that conflict, especially since as Anthony points out \n works (and then we don't need to do an extra level of parsing.
timhoffm commentedon Oct 16, 2020
My primary concern is: Which specification reads best and is most intuitive for our average user:
In my opinion, the first one wins by large. The concerns raised above are valid but don't outweigh that advantage.
Re: Added complexity
The amount of extra work compared to the \n-version is negligible - probably one line of code and two lines of docs.
Re: Relation to patchwork
I acknowledge that moasic is inspired by one specific part of patchwork namely the textual layout mechanism. It's only that. We don't have anything to do with patchwork composition operators (also explicitly stated in the tutorial: "While we do not implement the operator overloading style, we do provide a Pythonic API for specifying (nested) Axes layouts." ).
Patchwork does not allow single-line layout strings, so there cannot be a strong expectation how the line separation char should look like. If I'd have to define single-line layout string for patchwork, the row composition operator would be a strong suggestion. However, we have our own layout specification, which is only loosely related to patchwork; e.g. we chose to use a different empty placeholder by default.
Given the association would only be indirect over two steps (mpl layout --> patchwork layout --> patchwork compostion operator), I don't by that patchwork users would be significantly confused. Moreover, only a minority of our users will know patchwork anyway.
story645 commentedon Oct 16, 2020
It'd be the R users who already tend to be (understandably) frustrated that we have a different paradigm. I very much worry about the frustration of '|' means vertical alignment here and 'I' means horizontal alignment in the closest analogue they also might be using concurrently.
What about borrowing from the other layout and using "," so "AA,BB"?
timhoffm commentedon Oct 16, 2020
',' is easy to mix up with '.', which is our empty sentinel. A better variant would be
in analogy to multiple statements on the same line.
story645 commentedon Oct 16, 2020
I love the ';', it gets my vote!
timhoffm commentedon Oct 16, 2020
Reading below this line is at own personal risk. Just noted down some crazy ideas so that they don't get lost.
This is not an official API propsal.
At the danger of annoying everybody here again:
;as row separator almost calls for,as (optional) column separtor and allowing multi-char names in strings. At least for simple cases that'd be nice:Oh, or alternatively mapping single chars to long names: