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 colormap and pandas issues with pkg version updates #154

Merged
merged 3 commits into from
Jan 23, 2023

Conversation

jianheACM
Copy link
Collaborator

With matplotlib version 3.6+, matplotlib.cm.get_cmap is deprecated. There are some incompatibility with pandas with the latest version. I have made the fixes and they are now working properly.
However, we need to pay attention on this for the future updates. I'm worried we may have the similar issues in the future.

@zmoon
Copy link
Collaborator

zmoon commented Jan 3, 2023

some incompatibility with pandas with the latest version

To be clear, this was a bug in one pandas release, which has since been fixed, as we discussed in #135. If you install the latest pandas, there wouldn't be an issue.

we need to pay attention on this for the future updates

Yep, this is part of package maintenance.

melodies_monet/plots/surfplots.py Outdated Show resolved Hide resolved
melodies_monet/plots/surfplots.py Outdated Show resolved Hide resolved
melodies_monet/plots/surfplots.py Outdated Show resolved Hide resolved
@jianheACM
Copy link
Collaborator Author

@zmoon @rschwant I have updated the code as suggested. Could you please take a look?

@zmoon
Copy link
Collaborator

zmoon commented Jan 23, 2023

@jianheACM 4b9f5b4 is what I meant by my comment. Would you be able to try it out?

@rschwant
Copy link
Collaborator

For some reason, when I test using the develop_reg2 branch directly (Maybe this doesn't have the code completely set up?), for the bias plot I get the viridis colorbar instead of OrangeBlue? But I just realized, Zach you updated something, so let me retry it.

@rschwant rschwant self-requested a review January 23, 2023 22:13
Copy link
Collaborator

@rschwant rschwant left a comment

Choose a reason for hiding this comment

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

Thanks Zach and Jian for working on this! With your recent update now Zach, it works well and I'm no longer getting viridis as the colorbar, so I approve. @jianheACM as long as you are okay with Zach's changes, this can be merged.

@jianheACM
Copy link
Collaborator Author

@zmoon @rschwant I also tested on my side and it is working perfectly. Thanks!

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

Successfully merging this pull request may close these issues.

3 participants