Fix axis label clipping with PlotItem content margins (issue #3375) #3384
Conversation
This commit resolves GitHub issue pyqtgraph#3375 where rightmost tick labels were being clipped when using PlotItem with content margins. Changes include: - Improved default axis settings (hideOverlappingLabels=False for horizontal axes) - Enhanced text space calculation methods in AxisItem - Added PlotItem layout expansion support for axis text overflow - Cross-platform compatibility for different Qt layout APIs - Comprehensive error handling and backward compatibility The fix ensures autoExpandTextSpace works properly with content margins while maintaining full backward compatibility.
- Added test_AxisItem_clipping_fix_defaults() to verify improved defaults - Added test_AxisItem_text_space_methods() to test new text space calculation methods - Added test_AxisItem_clipping_with_plot_margins() to test the original issue scenario - Added test_AxisItem_manual_settings_override() to ensure backward compatibility - Added test_AxisItem_orientation_specific_behavior() to test all axis orientations - All tests integrated into existing tests/graphicsItems/test_AxisItem.py - All 27 tests passing including 5 new axis clipping tests
- Removed test_axis_clipping.py, test_axis_clipping_fix.py, test_fix_directly.py, validate_github_issue_fix.py - All test functionality has been properly integrated into tests/graphicsItems/test_AxisItem.py - Keeps repository clean and follows project test organization conventions
- Does not follow project conventions for documenting changes - CHANGELOG is the established format for tracking changes - Fix details should be in pull request description instead - Keeps repository clean and follows project standards
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if margins: | ||
| return margins | ||
| parent = parent.parentItem() | ||
| except (AttributeError, RuntimeError): |
Check notice
Code scanning / CodeQL
Empty except Note
| self._directLayoutExpansion(parent, required_space) | ||
| break | ||
| parent = parent.parentItem() | ||
| except (AttributeError, RuntimeError): |
Check notice
Code scanning / CodeQL
Empty except Note
| current_margins.bottom() | ||
| ) | ||
|
|
||
| except (AttributeError, TypeError): |
Check notice
Code scanning / CodeQL
Empty except Note
| axis = self.axes[axis_name]['item'] | ||
| if axis and hasattr(axis, '_onParentMarginsChanged'): | ||
| axis._onParentMarginsChanged(new_margins) | ||
| except (AttributeError, KeyError, RuntimeError): |
Check notice
Code scanning / CodeQL
Empty except Note
|
|
||
| # Notify axes of the change for better text space handling | ||
| self._notifyAxesOfMarginChange() | ||
| except (AttributeError, RuntimeError): |
Check notice
Code scanning / CodeQL
Empty except Note
| hide_overlapping_labels = True | ||
| # For horizontal axes, allow text to extend to prevent clipping | ||
| # This addresses the issue where rightmost labels get clipped | ||
| hide_overlapping_labels = False ## Changed from True - allow extension |
There was a problem hiding this comment.
I would suggest to leave hide_overlapping_labels unchanged. The justification for it being on for the horizontal axis is the following:
In the common application of an updating real-time chart, long timestamp labels move along the bottom axis. These have a tendency to stick out on the left and overlap the zero of the vertical axis. In a competition between the specific 0 label and an arbitrary timestamp, the 0 should take priority. That is not a strong reason, but enough to suggest not introducing a change in behavior.
A smart solution might want to distinguish between a side with potentially colliding axis labels ("left"), and a side where that is not the case ("right"), but I think we do not currently have the necessary functionality to either implement that in AxisItem, or to detect the best approach.
Until we find that smart solution to handle this automatically, the next step of progressive improvement might be to add more hints to the docs for how and why to manually set this flag?
| 'tickTextHeight': 18, | ||
| 'autoExpandTextSpace': True, ## automatically expand text space if needed | ||
| 'autoReduceTextSpace': True, | ||
| 'autoReduceTextSpace': auto_reduce_text_space, ## improved default based on orientation |
There was a problem hiding this comment.
This change makes sense to me, since the horizontal axis text space should not usually expand with label length, and therefore have less need to recover from temporary expansion.
| Parameters: | ||
| left, top, right, bottom (float): Margin values in pixels | ||
| """ |
There was a problem hiding this comment.
I have nothing to contribute to a discussion on how to best implement this...
But gaining more control over the label layout is an excellent addition.
| extra_needed = required_space - current_width | ||
|
|
||
| # Calculate new right margin (conservative approach) | ||
| additional_margin = min(extra_needed * 0.3, 50) # Cap at 50px max |
There was a problem hiding this comment.
Should we consider making the magic numbers into adjustable parameters? Especially if this might end up in a release without much testing period on master, it might break some application layouts in unexpected ways. Having a tweakable parameter would help with damage control in that case.
We can keep them as undocumented additions until we have more experience with the outcome and the urgency to make them accessible. Maybe PlotItem should acquire an axesLayout dictionary for this purpose.
|
Hi @pizofreude , hi @j9ac9k ! Regretfully, I do not have the time to dig into this PR properly, and I would not trust myself to override I also wanted to say that there is clearly room for improvement in how the labels function, and I am very happy to see someone work on that :) |
2 participants
Description
Fixes rightmost tick labels being clipped when using PlotItem with content margins.
Resolves issue #3375
Changes Made
Bug Fixes
hideOverlappingLabels=Falsefor horizontal axesautoReduceTextSpace=Falsefor better text preservationTesting
test_AxisItem.pyBackward Compatibility
Technical Details
AxisItemwith 4 new private methods for text space managementPlotItemlayout expansion methods with Qt version compatibility