🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@rcomer
Copy link
Member

@rcomer rcomer commented Jul 5, 2025

PR summary

Possibly closes #30230, though I'm unsure if it's a bit too hacky.

Introduce new private attributes so that apply_aspect can keep track of the previously set x/y-bounds. These attributes get reset when set_xlim/set_ylim are called, so further user updates to the limits are respected.

I ran the code from #30230 (comment) and messed about with the window shape. Everything seems fine.

If this seems good to others, I will add a test.

PR checklist

@QuLogic
Copy link
Member

QuLogic commented Sep 9, 2025

Did this not work?

@rcomer
Copy link
Member Author

rcomer commented Sep 9, 2025

It worked, but as I said above I wasn’t sure about it because it seemed a bit hacky. Since nobody commented, I figured everyone else was also not sure about it.

@rcomer
Copy link
Member Author

rcomer commented Sep 9, 2025

Possibly I could have labelled it differently to make it clearer I wanted feedback…

@rcomer rcomer reopened this Sep 9, 2025
@rcomer rcomer added status: needs comment/discussion needs consensus on next step and removed status: needs tests labels Sep 9, 2025
@timhoffm
Copy link
Member

timhoffm commented Sep 10, 2025

Random thoughts - sorry if this somewhat incoherent; I don't have a full overview of what is going on here either

I'm wondering whether we should strive for a more fundamental solution: The quantities xlim, ylim, aspect, box_aspect (explicit or via a fixed position box) over-specify the plot. Three out of four can be specified. The fourth is then induced. set_position already has the which parameter to distinguish between original and induced (active) value. Tracking that on every quantity seems impractical and I'd suspect that we won't be able to cover all combinations of given vs. induced parameters. Would it be more sustainable to track which of the quantities are set and at runtime calculate the real value? Though I'm not clear whether I'm overengineering this.


Fundamentally we have these cases

xlim / ylim aspect box_aspect
default (x) ~ x
set_aspect(..., adjustable="box") (x) x ~
set_aspect(..., adjustable="datalim") ~(*) x x

Legend:

  • ~ induced
  • x fixed / for limits: (x) fixed either by the user or inferred from data limits, but in both cases fixed as far as "induced" is concerned in the present context.

(*) The interesting part is in here. If data limits become variable, we must adapt one of xlim/ylim. And in theory one of them is enough. The bug is that we adjust both. My wild guess is that we keep one constant and adapt one as an induced effect to placate the above constraints. Then, constrained layout comes along and adjusts position/box_aspect, which results in another limit update, but that update does not know anymore that we intentionally modified one limit and not the other. Depending on the layout situation, it may adjust the other limit so that in the end both are modified.

@rcomer
Copy link
Member Author

rcomer commented Nov 22, 2025

Thank you for your thoughts @timhoffm. I have not been ignoring you, but every time I try to think about this one I get quite lost!

@timhoffm
Copy link
Member

@rcomer No worries. Same here, as my disclaimer above reveals. 🫣

@timhoffm
Copy link
Member

Thinking a bit more about it, I have now a more clear picture on the topic and a direction in which a solution must go.

Side-note: I just realized that set_aspect(..., adjustable="datalim") is a bit of a misnomer, it should actually be adjustable="viewlim". That may have added to the confusion. In the following, I use the generic term limits to not stumble over terminology.

  • default / no aspect: This is easy and works out of box as not requiring a data aspect means we have no coupling between the axes. The axes box and limits do not influence each other.
    No change required.
  • set_aspect(..., adjustable="box"): Aspect and limits are fixed. This implies a fixed box_aspect. Calculating the actual position (cosidering the requested box aspect) is done in the layout and tracked through the the original/active position mechanism. Note that the original/active position mechanism is more general as other properties of the plot influence positioining as well. It needed also beyond the aspect topic and seems to be an appropriate solution as is.
    No change required.
  • set_aspect(..., adjustable="datalim"): This is the complicated one. The data aspect is fixed, the box aspect is determined by the layout, the limits are calculated from these two quantities. We must be aware that every layout change, including resizing of the figure window will affect limits.
    A solution has to evolve around this concept. Tracking the bounds goes in that direction but I still have to think more on the details.

Next steps: We need to clarify the cases and desired limit behavior for the adjustable="datalim" case. When we know what we want, we can think of the proper technical measures to implement it.

IMHO there are three major cases to disinguish:

  • xlim and ylim are both fixed: The problem is overspecifed: We should error out. - t.b.d. when: at layout time, or can we do it already when introducting the overspecification in set_xlim/ylim/aspect? - Probalby easier at layout time
  • xlim and ylim are free: In this case, increase whatever direction is needed to get to the desired aspect.
  • one of xlim/ylim is fixed, the other is free. In the both-free case we could choose the direction to modify and therefore only ever "increase" view limits starting from the data limits, i.e. there's always a solution that still shows all data. This is not the case anymore with one fixed direction. To fulfill the aspect it may be needed to increase or decrease the free direction. Decreasing means we would have to cut data from the view. We have three possible ways of handling this:
    • Cut data (possible with a warning?)
    • Completely disallow the one-fixed-one-free case and error out like in both-fixed
    • Do a case-by-case decision: If expanding the limit is needed, do it, if shrinking the limit is needed, error out. For a static plot this seems like the best option. But giben that figure resizing may switch between the two cases, this would mean, resizing could lead to an error. I don't think this is acceptable behavior. Therefore, we should go with one of the two other variants.

@rcomer
Copy link
Member Author

rcomer commented Nov 23, 2025

  • Do a case-by-case decision: If expanding the limit is needed, do it, if shrinking the limit is needed, error out. For a static plot this seems like the best option.

I think even for static plots this would not work well in general unless we can somehow make dataLim account for clipping. Taking the simple case from #20380 (comment), once we fix the x-limits, the visible y-extent of the line is reduced. The user might reasonably expect the automatic y-limits to be centered on the remaining visible portion of the line.

@timhoffm
Copy link
Member

Checking the code, we currently may ignore explicitly set limits to ensure we are always increasing the range and not clipping data.

if adjust_y:
yc = 0.5 * (ymin + ymax)
y0 = yc - Ysize / 2.0
y1 = yc + Ysize / 2.0
if not self.get_autoscaley_on():
_log.warning("Ignoring fixed y limits to fulfill fixed data aspect "
"with adjustable data limits.")
self.set_ybound(y_trf.inverted().transform([y0, y1]))
else:
xc = 0.5 * (xmin + xmax)
x0 = xc - Xsize / 2.0
x1 = xc + Xsize / 2.0
if not self.get_autoscalex_on():
_log.warning("Ignoring fixed x limits to fulfill fixed data aspect "
"with adjustable data limits.")
self.set_xbound(x_trf.inverted().transform([x0, x1]))

While one could debate whether that's the best behavior, it at least is a clear and consistent solution; and since it's currently there, I'm ok with leaving it as is. This removes a lot of complication as the one-fixed case is logically the same as both-free (only that we issue a warning if we ignore the fixed limit).

This means, we only have to solve the both-free case, in particular the change of limits/bounds through change of axes box size. And yes, I agree with the PR in that we have to track whether the bounds have been explicitly set or are an induced effect. We have to re-check how change of data limits interacts with this: What happens if I add new data points after the layout has done? Does the addition of new data result in bounds recalculation when a new layout is done? - It should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Neither explicit data limit is respected when constrained_layout is used together with set_aspect(..., "datalim")

3 participants