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

copy coords before modifying #2160

Merged
merged 2 commits into from
Nov 15, 2022
Merged

Conversation

tspeng
Copy link
Contributor

@tspeng tspeng commented Nov 11, 2022

Description

Make a copy of the dictionary coords, so the input parameter coords is not modified.

This PR is to fix the issue mentioned in issue #2047


📚 Documentation preview 📚: https://arviz--2160.org.readthedocs.build/en/2160/

@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Merging #2160 (0735687) into main (6e8c8ab) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 0735687 differs from pull request most recent head 8a91839. Consider uploading reports for the commit 8a91839 to get more accurate results

@@           Coverage Diff           @@
##             main    #2160   +/-   ##
=======================================
  Coverage   90.68%   90.68%           
=======================================
  Files         120      120           
  Lines       12678    12679    +1     
=======================================
+ Hits        11497    11498    +1     
  Misses       1181     1181           
Impacted Files Coverage Δ
arviz/plots/ppcplot.py 89.33% <100.00%> (+0.14%) ⬆️
arviz/stats/__init__.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ahartikainen
Copy link
Contributor

Small update to changelog and this is ready

@OriolAbril OriolAbril linked an issue Nov 15, 2022 that may be closed by this pull request
@OriolAbril
Copy link
Member

Same comment as @ahartikainen. Thanks for the PR. You only need to add one line to https://github.com/arviz-devs/arviz/blob/main/CHANGELOG.md#maintenance-and-fixes and it will be ready to merge.

@tspeng
Copy link
Contributor Author

tspeng commented Nov 15, 2022

@ahartikainen and @OriolAbril Thank you for the comments. I just added one line to the changelog. Let me know if it works.

@ahartikainen
Copy link
Contributor

ahartikainen commented Nov 15, 2022

Thanks, everything looks good.

I cleaned the commits a bit using git fetch --all and git rebase -i upstream/main (so you get the latest changes without including the commits, assuming you have upstream defined).

@ahartikainen ahartikainen merged commit 8be27b8 into arviz-devs:main Nov 15, 2022
@tspeng
Copy link
Contributor Author

tspeng commented Nov 16, 2022

@ahartikainen Thank you for making the PR much cleaner. Just for a learning purpose, how did you choose the keywords after "git rebase -i upstream/main" ? I guess you "pick" the two remaining commits. Did you drop the other two commits?

@ahartikainen
Copy link
Contributor

Yes

@tspeng
Copy link
Contributor Author

tspeng commented Nov 16, 2022

Got it, thanks a lot!

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.

plot_ppc modifies coords argument
5 participants