Domain error uncaught when using padding and row heights

[stephenpardy]

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

ValueError: 
    Invalid value of type 'builtins.float' received for the 'domain[0]' property of layout.yaxis
        Received value: 1.0078125

    The 'domain[0]' property is a number and may be specified as:
      - An int or float in the interval [0, 1]

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:

from plotly.subplots import make_subplots

n_subplots = 8
padding_size = 0.2

specs = []
for _ in range(n_subplots):
    specs.append([{'b': padding_size/2., 't': padding_size/2.}])
    
fig = make_subplots(rows=n_subplots, specs=specs)

fig

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.

[AaronStiff]

b and t are added to each cell here:

plotly.py/packages/python/plotly/plotly/_subplots.py

Lines 698 to 705 in 743b524

	# Get y domain (dep. on row_dir) using grid & r_spanned
	if row_dir > 0:
	y_s = grid[r][c][1] + spec["b"]
	y_e = grid[r_spanned][c][1] + heights[r_spanned] - spec["t"]
	else:
	y_s = grid[r_spanned][c][1] + spec["b"]
	y_e = grid[r][c][1] + heights[-1 - r] - spec["t"]
	y_domain = [y_s, y_e]

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."
    ) 

[stephenpardy]

That would definitely make the error a bit clearer for the user.

[stephenpardy]

@AaronStiff I can take this on as a PR if you are open to that.

[nicolaskruchten]

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.

[stephenpardy]

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.

[nicolaskruchten]

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.

[AaronStiff]

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.

[nicolaskruchten]

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 :)

[AaronStiff]

Very true, I'll admit I just copied the previous exception message :)

[nicolaskruchten]

Hehe I see! Well, we can probably improve those too along the way :)

[stephenpardy]

Drafting PR here: #4153

[gvwilson]

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