Skip to content
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

Make plots resize with windows better #35195

Merged
merged 14 commits into from
Mar 10, 2023
Merged

Conversation

jhaigh0
Copy link
Contributor

@jhaigh0 jhaigh0 commented Feb 10, 2023

Description of work.

This PR fixes a bug where plot axes labels could be hidden outside the window boundaries after resizing. It also aims to make all plot types resize in a more predictable and nicer way.

An important part of this was giving plots a tight layout; this has resulted in warnings being printed to the console when a window is resized to be very small. Since these warnings are annoying, I tried to filter them out, but couldn't find a solution that worked. However, you have to shrink the plot to an unusable size to make them appear, so I think it is okay overall.

To test:

To test these changes, it will be useful to have another version of Mantid open to see the differences.

  • Load 164199.nxs (from the training course data)
  • Right-clicking on the loaded workspace, plot a....
    • Spectrum
    • Spectrum with errors
    • Spectrum range tiled plot
    • Spectrum range waterfall plot
    • Super Plot
    • Super Plot with errors
    • Colorfill
    • Surface
    • Wireframe
    • Contour
  • And for each of these resize the window to a few different reasonable sizes and check that
    1. No figure objects are colliding
    2. No parts of the figure are being hidden outside the window boundaries
    3. Moving the legend and editing axes names and ranges doesn't break anything
  • Now load 164198.nxs and 164200.nxs
  • Group the three workspaces together (highlight all and click Group)
  • Right-clicking on the new group, plot all the different types of plots again and make the same checks

Finally, test with a some other graphing scenarios that you think might be challenging.

Fixes #35014


Reviewer

Please comment on the following (full description):

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards?
  • Are the unit tests small and test the class in isolation?
  • If there is GUI work does it follow the GUI standards?
  • If there are changes in the release notes then do they describe the changes appropriately?
  • Are the release notes saved in a separate file, using Issue or PR number for file name and in the correct location?
Functional Tests
  • Do changes function as described? Add comments below that describe the tests performed?
  • Do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

@jhaigh0 jhaigh0 added GUI Issues and pull requests specific to the Mantid Workbench GUI. ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS labels Feb 10, 2023
@jhaigh0 jhaigh0 added this to the Release 6.7 milestone Feb 10, 2023
@sf1919 sf1919 self-assigned this Feb 27, 2023
Copy link
Contributor

@sf1919 sf1919 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works very well and it's useful that when the labels etc are cut off you do get an error message - Tight layout not applied. The left and right margins cannot be made large enough to accommodate all axes decorations.

I've asked for a couple of changes to the comments just to make them clearer.

Should this have test(s)?

Framework/PythonInterface/mantid/plots/plotfunctions.py Outdated Show resolved Hide resolved
if add_cbar_axis:
# The right most column of the GridSpec is made a SubGridSpec to facilitate the colour bar
# This is done so that the colour bar can have a close spacing to the right most column
# Keeping the colour bar in the GridSpec rather than alongside it means it behaves much nicer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be reworded slightly? It seems a bit subjective and not clear what behaviour it is actually doing.

jhaigh0 and others added 2 commits March 6, 2023 12:07
Co-authored-by: Sarah Foxley <55837273+sf1919@users.noreply.github.com>
@jhaigh0
Copy link
Contributor Author

jhaigh0 commented Mar 7, 2023

Yes, I reckon I could've put a test in for the creating subplots colour bar change; I'll add that.

Copy link
Contributor

@sf1919 sf1919 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting the tests in. I think they're a good addition and should hopefully catch anything like this in future.

This one is now good to go.

@DannyHindson DannyHindson merged commit f63b4ed into main Mar 10, 2023
@DannyHindson DannyHindson deleted the 35014_plot_labels_being_hidden branch March 10, 2023 15:11
@jhaigh0 jhaigh0 mentioned this pull request Apr 26, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI Issues and pull requests specific to the Mantid Workbench GUI. ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

X Axis Title of the graphs hides when we change size (second fix)
3 participants