Change dispersion plot ylimit handling #107
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the case where a dispersion exhibits imaginary frequency modes or the user specifes bandmin, the current logic sets the bottom ylim as the minimum frequency of the data. Removing this else condition lets matplotlib handle white space padding automatically in order to not cut off the band(s).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
In the case where a dispersion exhibits imaginary frequency modes or the user specifies bandmin, the current plotting workflow does not leave any visual room at the edge of the plot which can slightly cut off the band(s). All in all, not a big deal, but I think it's visually nicer to have a little margin.
Note: it seems that having a little margin was the default behavior before #104
Solution:
For
add_dispersion()andadd_projected_dispersion(), I removed the "else" condition and let matplotlib handle margin padding automatically.For
add_wideband(), I (manually) added matplotlib's default 5% margin padding to both the bottom and the top of the plot with two exceptions:You can see a small change at the top of the plot-wideband example. The 5% margin might not actually be big enough in the cases where the phonon (physical) linewidths due to scattering are large, although this could be adjusted by passing ymax as an argument.
Further comments:
There might still be an issue for
add_alt_dispersion()oradd_alt_projected_dispersion()as I didn't have time to fully investigate these, but I figured I could at least start with this pull request as it is now. I'd be happy to make any changes or incorporate any feedback you might have!