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

Fix animate_list blank figures #80

Merged
merged 6 commits into from
Dec 12, 2019

Conversation

rdoyle45
Copy link
Collaborator

We will likely change this in the future. But for now this will remove blank subplots from figure.

@pep8speaks
Copy link

pep8speaks commented Dec 11, 2019

Hello @rdoyle45! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-12-12 01:56:14 UTC

@rdoyle45 rdoyle45 added the bugfix Fix for a bug label Dec 11, 2019
@codecov-io
Copy link

codecov-io commented Dec 11, 2019

Codecov Report

Merging #80 into master will increase coverage by 0.44%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #80      +/-   ##
=========================================
+ Coverage   47.05%   47.5%   +0.44%     
=========================================
  Files          11      11              
  Lines        1052    1061       +9     
  Branches      210     212       +2     
=========================================
+ Hits          495     504       +9     
  Misses        496     496              
  Partials       61      61
Impacted Files Coverage Δ
xbout/plotting/animate.py 42.96% <66.66%> (ø) ⬆️
xbout/boutdataset.py 66.41% <90.9%> (+2.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75c108b...bdd3799. Read the comment docs.

@@ -209,10 +212,12 @@ def animate_list(self, variables, animate_over='t', save_as=None, show=False, fp
"""

nvars = len(variables)
std_format = False
Copy link
Collaborator

@johnomotani johnomotani Dec 12, 2019

Choose a reason for hiding this comment

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

I think it makes sense to remove the blank figures even when nrows and/or ncols are given - I think it's unlikely that the blank axes are useful. So we can remove the std_format condition.

Even if 'nrows' and/or 'ncols' was given explicitly, it makes sense to delete
any blank axes with no plot, so remove the std_format flag.
@johnomotani johnomotani merged commit 7f24c16 into boutproject:master Dec 12, 2019
@rdoyle45 rdoyle45 deleted the fix-animate_list-scaling branch December 12, 2019 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants