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

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

Closed
1 of 2 tasks
EwoutH opened this issue May 13, 2022 · 4 comments · Fixed by #317
Closed
1 of 2 tasks

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

EwoutH opened this issue May 13, 2022 · 4 comments · Fixed by #317
Milestone

Comments

@EwoutH
Copy link
Collaborator

EwoutH commented May 13, 2022

Instead of only inspecting one point, it would be neat if you can pass multiple points into the inspect() function of an analysis.prim.PrimBox. The expected behavior could be:

  • For PrimBox.inspect(i=list, style="table"), a dataframe with a row for each point will be provided.
  • For PrimBox.inspect(i=list, style="graph"), either Figure with a subplot for each point could be provided, or a single graph with the multiple points compared to each other (each point with a different color bars).

This will allow to more quickly inspect and compare multiple points on the peeling trajectory.

Also I noted that the inspect function prints a lot of data without returning it. I think it's nicer and more expected to return the values/tables/graphs, and let the user print them. That way you can use them in further analysis if you want.

@quaquel
Copy link
Owner

quaquel commented Jun 1, 2022

Implementing this is largely straightforward. Basically move most of the current code in inspect into some protected function _inspect, and then possibly loop over this depending on whether i is an integer or a list.

If we want to also control for style='graph' whether to show each box as a separate figure or have them in a single figure, then things become messier. My hunch is that it would actually break the current visualization, because of overplotting the values of the box limits as well as having to handle multiple quasi-p values (one for each box lim).

Not sure what you mean exactly by your last remark. Given that the printing to screen is a bit more involved than simply creating the DataFrame (see line 455 in prim.py ). There is a usecase for returning a DataFrame-like representation of the box lims which you can then send this to the clipboard for use in a report or something. I would, however, be inclined to treat that as a third option for the style kwarg.

@EwoutH
Copy link
Collaborator Author

EwoutH commented Jun 1, 2022

If we want to also control for style='graph' whether to show each box as a separate figure or have them in a single figure, then things become messier.

I think it would be nice to create subplot for each point and their box, and then return a single figure. We could set a default number of subplots that fit next to each other horizontally, 3 for example, and extend vertically as more points get plotted. I can work on this piece a bit.

Not sure what you mean exactly by your last remark.

You got it almost exactly right :). I like the idea of having separate style kwargs for printing and returning data. I think a set/tuple/dictionary of DataFrames might be the way to go.

@quaquel
Copy link
Owner

quaquel commented Jun 1, 2022

The easiest next step for the visualization would be to modify scenario_discovery_util.plot_box to take an optional axes instance as a kwarg (similar to how you can do this in seaborn). scenario_discovery_util._setup_figure then needs to be modified (and renamed) to operate on a provided. A quick check of the code suggests that with those changes, all you need to do is set up a figure with arbitrary rows and cols and simply iterate over the resulting axes and plot a single box in each axes.

quaquel added a commit that referenced this issue Jun 1, 2022
First step for addressing #124. inspect now takes an integer or list of integers. It raises a TypeError if i is not an integer or list of integers. Test code is updated to reflect these changes.
quaquel added a commit that referenced this issue Jun 3, 2022
next step for #124. inspect now has a third option which returns a list of tuples where each tuple contains the stats and box lims for the corresponding entry in i.
@EwoutH EwoutH added this to the 2.3.0 milestone Aug 31, 2022
@EwoutH
Copy link
Collaborator Author

EwoutH commented Oct 28, 2022

Let's move the "graph" part to 2.4.0, as part of a larger plotting overhaul.

@EwoutH EwoutH modified the milestones: 2.3.0, 2.4.0 Oct 28, 2022
@quaquel quaquel modified the milestones: 2.4.0, 2.5.0 Apr 19, 2023
quaquel added a commit that referenced this issue Nov 18, 2023
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 added a commit that referenced this issue Dec 4, 2023
* make it possible to plot mulitple boxes in one figure

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.

closes #124

* Update test_prim.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants