I think Jae-Joon's new figure refactoring is really great, and is a huge improvement over the old code.
I do, however, have one concern. Since the legend border is no longer a simple rectangle, the Agg backend's auto-pixel-alignment routine is no longer kicking in, and the border ends up looking fairly fuzzy. It's particularly striking when the axes rectangle is always so clean and sharp now. As nice as the rounded edges are, the fuzziness makes it feel like a bit of a regression to me. Aesthetically, I also feel it's a bit out of place to use rounded corners in one case but not elsewhere in a document (typical LaTeX templates, for instance, don't use rounded corners.)
Should it perhaps be a regular rectangle by default, with rounded corners as an option?
This may also be an argument for finally making the auto-pixel-alignment code programmatic, rather than automatic. As it works now, it automatically pixel-aligns when there are a) no curves and b) only rectilinear axis-aligned line segments. If the pixel alignment was instead controlled from Python as a flag, then the legend code could explicitly say it wants the legend patch to be pixel-aligned, even if it has rounded corners. But that's perhaps too deep of a change to make for the impending release and should have to wait for next time.
Mike
···
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA
I think Jae-Joon's new figure refactoring is really great, and is a huge
improvement over the old code.
I do, however, have one concern. Since the legend border is no longer a
simple rectangle, the Agg backend's auto-pixel-alignment routine is no
longer kicking in, and the border ends up looking fairly fuzzy. It's
particularly striking when the axes rectangle is always so clean and
sharp now. As nice as the rounded edges are, the fuzziness makes it
feel like a bit of a regression to me. Aesthetically, I also feel it's
a bit out of place to use rounded corners in one case but not elsewhere
in a document (typical LaTeX templates, for instance, don't use rounded
corners.)
Should it perhaps be a regular rectangle by default, with rounded
corners as an option?
I'm inclined to agree with you -- this could also be an rc option, so
those who like the rounding can get them by default. I would be happy
to have rectangle by default, with liberal use of rounding in the
gallery example code so people can easily find how to turn them on.
A legend section for the docs is still on my list of things to do.
I'll implement the rc param "legend.fancybox" and make False
(rectangle) the default, and we can always change the after default if
JJ weighs in.
This may also be an argument for finally making the auto-pixel-alignment
code programmatic, rather than automatic. As it works now, it
automatically pixel-aligns when there are a) no curves and b) only
rectilinear axis-aligned line segments. If the pixel alignment was
instead controlled from Python as a flag, then the legend code could
explicitly say it wants the legend patch to be pixel-aligned, even if it
has rounded corners. But that's perhaps too deep of a change to make
for the impending release and should have to wait for next time.
This sounds fairly benign (gc param?) so I could go either way: before or after.
JDH
···
On Mon, Dec 8, 2008 at 9:13 AM, Michael Droettboom <mdroe@...31...> wrote:
I wasn't thinking clearly. I was thinking you would be applying the
fix only for the legend implementation, but there is no clean way to
do this since the backends don't know anything about legends. Yes,
doing this for all the artists is definitely too ambitious at this
stage of the release.
JDH
···
On Mon, Dec 8, 2008 at 9:39 AM, Michael Droettboom <mdroe@...31...> wrote:
The gc param would be the easy part -- I was thinking the difficult would be
going through all the cases and making sure it's doing the right thing, and
making sure axes and ticks etc. still look nice. Though I suppose having
the flag be yes/no/auto (where auto is the current behavior) would be the
easiest route.
John Hunter wrote:
The gc param would be the easy part -- I was thinking the difficult would be
going through all the cases and making sure it's doing the right thing, and
making sure axes and ticks etc. still look nice. Though I suppose having
the flag be yes/no/auto (where auto is the current behavior) would be the
easiest route.
I wasn't thinking clearly. I was thinking you would be applying the
fix only for the legend implementation, but there is no clean way to
do this since the backends don't know anything about legends. Yes,
doing this for all the artists is definitely too ambitious at this
stage of the release.
I think this also dovetails nicely with another update I've been meaning to make -- to use a GC (or something like it) for images and collections as well. As it stands now, any new property to collections needs to be added as a new argument to every backend (as was recently done for hyperlink support). Stuffing everything in an object should make it more easily extensible.
But that's not before the release... 
Mike
···
On Mon, Dec 8, 2008 at 9:39 AM, Michael Droettboom <mdroe@...31...> wrote:
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA
legend.fancybox=False as the default is good.
Just in case, the legend class still creates FancyBboxPatch(but with a
rectangle style) even if fancybox=False. But, I presume that the
auto-pixel-alignment works in this case.
On the other hand, I think it would also good if we allow a dictionary
for the "fancybox" argument, like "bbox" in text. But maybe not in
this release.
Regards,
-JJ
···
On Mon, Dec 8, 2008 at 10:27 AM, John Hunter <jdh2358@...149...> wrote:
I'm inclined to agree with you -- this could also be an rc option, so
those who like the rounding can get them by default. I would be happy
to have rectangle by default, with liberal use of rounding in the
gallery example code so people can easily find how to turn them on.
A legend section for the docs is still on my list of things to do.
I'll implement the rc param "legend.fancybox" and make False
(rectangle) the default, and we can always change the after default if
JJ weighs in.
I would prefer not to change the meaning of the fancybox once we
release it, if we can help it. The best two choices are:
* add support now for the dict before the release (handling the rc
to accept a dict may require a little extra work)
* keeping fancybox as is but adding an optional fancyprops dict later
I can go either way, but I am disinclined to intentionally change the
semantics of fancybox once we have released it we can help it
···
On Mon, Dec 8, 2008 at 10:49 AM, Jae-Joon Lee <lee.j.joon@...149...> wrote:
On the other hand, I think it would also good if we allow a dictionary
for the "fancybox" argument, like "bbox" in text. But maybe not in
this release.
I see your point.
Let's keep it simple and leave it as is.
I guess there might not be much need of customization as most of users
will be happy with the default behavior.
Regards,
-JJ
···
On Mon, Dec 8, 2008 at 12:30 PM, John Hunter <jdh2358@...149...> wrote:
On Mon, Dec 8, 2008 at 10:49 AM, Jae-Joon Lee <lee.j.joon@...149...> wrote:
On the other hand, I think it would also good if we allow a dictionary
for the "fancybox" argument, like "bbox" in text. But maybe not in
this release.
I would prefer not to change the meaning of the fancybox once we
release it, if we can help it. The best two choices are:
* add support now for the dict before the release (handling the rc
to accept a dict may require a little extra work)
* keeping fancybox as is but adding an optional fancyprops dict later
I can go either way, but I am disinclined to intentionally change the
semantics of fancybox once we have released it we can help it