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

inspect multiple boxes and display them in a single figure #317

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

quaquel
Copy link
Owner

@quaquel quaquel commented Nov 20, 2023

closes #124

Adds an optional keyword argument ax to PrimBox.inspect to be used in conjunction with style='graph'. Ax can be a single Axes or a list of Axes instances of the same length as i. If used, each box in i is plotted in separate Axes.

Rather that try to solve the axes layout as suggested in #124, it is left to the user to setup the figure to their liking. See sd_prim_flu.py for an example usage.

default behavior of inspect with list of indices is to show each box in a seperate figure (if style is graph). This code makes it possible to plot them in a single figure.

outstanding issue from #124
@quaquel quaquel requested a review from EwoutH November 20, 2023 19:31
@quaquel quaquel added this to the 2.5.0 milestone Nov 20, 2023
@coveralls
Copy link

coveralls commented Nov 20, 2023

Coverage Status

coverage: 80.251% (+0.05%) from 80.206%
when pulling 55b5a44 on inspect_graph
into 2002532 on master.

@EwoutH
Copy link
Collaborator

EwoutH commented Nov 21, 2023

Thanks for implementing this, I will try to review it this week.

@quaquel
Copy link
Owner Author

quaquel commented Dec 1, 2023

Just a simple reminder that it would be great if you could take a quick look at this so I can merge it.

@EwoutH
Copy link
Collaborator

EwoutH commented Dec 1, 2023

Thanks, I put it on my list for Monday.

To give some context, I'm now really in a hard push to get my thesis started. As always, the things I find most interesting are also the hardest and biggest. Then this requires to get a full overview of the system to properly identify the gaps - and then selecting a feasonable one. So that consumes most of my energy right now.

Also, maybe it would be good to talk about my formal role in the workbench maintenance again. When I was TAing in EPA1361 and after that working on the MPIEvaluator is was a relatively low effort to do it on the side (and a great learning experience - it still is), I now notice takes a bit more effort to switch to it.

Copy link
Collaborator

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

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

Looks great! I really like the warnings for catching invalid input.

Only thing I would like to see is another test with multiple boxes and axes, further everything looks good to me!

@quaquel quaquel merged commit 2ae0278 into master Dec 4, 2023
21 of 22 checks passed
@quaquel quaquel deleted the inspect_graph branch December 4, 2023 18:15
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.

[Feature] Support inputting multiple points (in a list) with PrimBox.inspect()
3 participants