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

Squeeze out space in profile plots #126

Merged
merged 2 commits into from
Aug 15, 2021
Merged

Squeeze out space in profile plots #126

merged 2 commits into from
Aug 15, 2021

Conversation

charleskawczynski
Copy link
Member

This PR updates the layout of the profile plots to squeeze out the lost white space. I also changed the layout to be 5 columns (it was previously 4). We can change it back if people prefer 4, most of the gained space was in the horizontal direction, so I think this actually preserves the look of the figures over sticking with 4, but I have no strong feelings about this.

Closes #124.

@haakon-e
Copy link
Member

Could you add a top-level title which states which simulation case the subplots are showing? e.g. "Bomex contours", etc.

Also, I think we should consider making all plots larger (extend width & height). If I zoom in to study details of one profile, it becomes apparent that the resolution could be improved.
image
In safari, this also seems to be maximum zoom level.
I realize you may have chosen this size to fit all subplots in a typical browser window, but I'm not sure if this is strictly necessary.

Copy link
Member

@haakon-e haakon-e left a comment

Choose a reason for hiding this comment

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

Other than that, LGTM!

@charleskawczynski
Copy link
Member Author

Ahhh, I see what you mean, we’ll need to increase the size and font size, since increasing the size has the effect of making the font look smaller. I’ll try this out soon

@charleskawczynski
Copy link
Member Author

Alright, updated! Would be nice to hear feedback on these changes when people get a chance. I think it's looking near complete 🚀

@haakon-e
Copy link
Member

Seems like you need to pass through the case name to the plotting functions.

@charleskawczynski
Copy link
Member Author

Seems like you need to pass through the case name to the plotting functions.

Whoops, updated, hopefully this works.

@charleskawczynski charleskawczynski force-pushed the ck/improve_profiles branch 2 times, most recently from 991ad9d to eec9571 Compare August 15, 2021 17:33
@charleskawczynski
Copy link
Member Author

Going to merge this for now. I think we can do tweaking beyond this if people wish.

@charleskawczynski
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 15, 2021

@bors bors bot merged commit 6f4a43f into main Aug 15, 2021
@bors bors bot deleted the ck/improve_profiles branch August 15, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve profile plots
2 participants