Closed JeffFessler closed 2 years ago
This seems like a convoluted corner case to me that might not be worth the effort to fix, but maybe it's just me? 🤷♂️
Why would one use different units if they intend to set the aspect ratio to equal
? Should this throw when the units of the x and y axes are incompatible?
In the actual situation where I hit on this, my units happened to be mm
and cm
, which seems less convoluted than using both inches and cm. I am sure there are more important issues to fix elsewhere, but I just wanted to document it.
Personally I would prefer at most @warn
rather than throw
for incompatible units.
Personally I would prefer at most
@warn
rather thanthrow
for incompatible units.
Ah I would have thought you'd want it to throw in this case...
Anyway, I think you are right: It's good to document this! Maybe we can add a warning in the docs to say that aspect_ratio
works on unit-stripped values rather than unitful values?
In theory, the aspect ratio for a unitful plot should be a unitful number (Hz/cm
), with :equal
corresponding to 1
.
Agreed, so now I also agree that :equal
should throw for incompatible units.
I might still wish for a different option name to handle cm/mm
with physically meaningful aspect ratio but that's probably not worth the effort. Just having a warning in the docs seems OK.
Closed in #77
The following MWE makes a 2inch by 2inch square, using a mix of inches units and mm units for the two axes.
However, all four options for
aspect_ratio
, i.e.,[1, :auto, :equal, :none]
produce a rectangle rather than a square, because1
andequal
make the spacing of the digits equal, rather than the physical units. (Note the grid lines make squares for1
andequal
.)Here is the resulting plot using UR v1.5.3 and Plots v1.13.1:
In the long run, it might be preferable to have some option (perhaps
equal
) foraspect_ratio
that makes the actual distances match (when the units are compatible). I have no idea whether it can be done here in UR or would require downstream changes in Plots.In the short run, this issue at least will document the behavior. It would be easy to at least make the current
heatmap
example more informative by having axes with units:https://jw3126.github.io/UnitfulRecipes.jl/stable/examples/2_Plots/#Heatmap,-categorical-axes,-and-aspect_ratio