-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Domain error uncaught when using padding and row heights #4124
Comments
plotly.py/packages/python/plotly/plotly/_subplots.py Lines 698 to 705 in 743b524
Perhaps an appropriate exception before that last line such as the following? if y_s > 1.0:
raise Exception(
"Some 'b' value is too large for " "this subplot gird."
)
if y_e > 1.0:
raise Exception(
"Some 't' value is too large for " "this subplot grid."
) |
That would definitely make the error a bit clearer for the user. |
@AaronStiff I can take this on as a PR if you are open to that. |
I wonder if some kind of very slight rounding wouldn't also be appropriate here... if the final number is extremely close to 1, like within a hundredth, which will often be on the order of one pixel in output, we could just internally round it down to 1 and avoid the error altogether. |
Great idea. That would be better in most situations - or a slight nudging of all of the padding? I do hesitate to mess with the user settings if they are passing in the arguments that cause these errors. But that requires providing users with a clear message of why their inputs are leading to errors as well. |
Well in this case the actual "why" is floating point error accumulation I think, which is painful to push onto a user and can easily be fixed by rounding. But yes for larger errors we should indicate that all the t's and b's and heights add up to more than 1. |
Yeah, a bit of rounding makes sense, maybe something like if y_s > 1.0:
if y_s < 1.01:
y_s = 1.0
else:
raise Exception(
"Some 'b' value is too large for " "this subplot gird."
)
if y_e > 1.0:
if y_e < 1.01:
y_e = 1.0
else:
raise Exception(
"Some 't' value is too large for " "this subplot grid."
) @stephenpardy you're more than welcome to create a pull request for this. You could even just open it as a draft PR for discussion and until we get some tests in place. |
Something like that would be good but I would be careful with the phrasing: it's not "some b" which is too big, it's the combination of t's and b's and heights which in isolation are each likely fine but together cause problems :) |
Very true, I'll admit I just copied the previous exception message :) |
Hehe I see! Well, we can probably improve those too along the way :) |
Drafting PR here: #4153 |
Hi - we are tidying up stale issues and PRs in Plotly's public repositories so that we can focus on things that are still important to our community. Since this one has been sitting for a while, I'm going to close it; if it is still a concern, please add a comment letting us know what recent version of our software you've checked it with so that I can reopen it and add it to our backlog. If you'd like to submit a PR, we'd be happy to prioritize a review, and if it's a request for tech support, please post in our community forum. Thank you - @gvwilson |
I am using plotly
5.13.1
on an M1 Mac.I have found an issue where setting the padding and row heights of multiple subplots will generate an error
This error has been previously reported when making many subplots. See #2556, #2026, #2273, #1216, #1121, #1031. In those cases the issue was related to
vertical_spacing
or other parameters, and an explicit error is now raised when users try to set spacings that would crash the plotting.In my case, I am setting the padding (and/or row heights) and get the original domain error.
Reproducible example:
I see in previous issues that the grid is in a square of [0, 1], [0, 1], but the padding sizes here only cause the error when the number of rows is > 7 which doesn't seem to follow that pattern.
I would love it if we could get a similarly descriptive error message for these cases and some guidance on where exactly the issue is coming from so that I can work on a workaround for our plotting code.
The text was updated successfully, but these errors were encountered: