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

Add aesthetic flexibility to plot_rm_corr #312

Merged
merged 3 commits into from
Nov 24, 2022

Conversation

remrama
Copy link
Contributor

@remrama remrama commented Nov 22, 2022

Hi @raphaelvallat - I love the plot_rm_corr function and needed some flexibility to change its scatter and line features (e.g., marker size and line width). I made a simple change to allow this, adding two sets of kwargs that get passed through to seaborn. Nothing about the defaults have changed, it just offers flexibility if desired.

Sorry I didn't open an Issue first, it was just such a straight-forward change I thought I'd go for it. No pressure if you want to reject this idea.

@remrama remrama marked this pull request as draft November 22, 2022 07:57
@raphaelvallat raphaelvallat added the feature request 🚧 New feature or request label Nov 23, 2022
@raphaelvallat
Copy link
Owner

Hi @remrama,

Thanks for opening the PR! Looks great. Two things:

  • Can you please remove pytest-travis-fold from the requirements-test.txt file? This should solve the CI failure.
  • Can you please add this to the changelog? Under a new section "Improvements"?

@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

Base: 98.70% // Head: 98.70% // No change to project coverage 👍

Coverage data is based on head (e0bbc23) compared to base (bc79523).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #312   +/-   ##
=======================================
  Coverage   98.70%   98.70%           
=======================================
  Files          18       18           
  Lines        3312     3312           
  Branches      537      537           
=======================================
  Hits         3269     3269           
  Misses         25       25           
  Partials       18       18           
Impacted Files Coverage Δ
pingouin/plotting.py 96.50% <100.00%> (ø)

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@raphaelvallat
Copy link
Owner

Let me know when the PR is ready for review/merge 👍

@remrama
Copy link
Contributor Author

remrama commented Nov 24, 2022

Yep it's good for review! I made the suggested changes.

@remrama remrama marked this pull request as ready for review November 24, 2022 19:46
@raphaelvallat raphaelvallat merged commit 628fd53 into raphaelvallat:master Nov 24, 2022
@raphaelvallat
Copy link
Owner

Merged, thanks!

@remrama remrama deleted the plot_rm_corr_flexibility branch November 25, 2022 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request 🚧 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants