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

Adding Grid.tick_params() method. #2944

Merged
merged 4 commits into from
Aug 10, 2022

Conversation

stefmolin
Copy link
Contributor

@stefmolin stefmolin commented Aug 7, 2022

Closes #2491

FacetGrid and PairGrid now have a tick_params() method, which will iterate over each Axes in the grid and call its tick_params() method. Some examples:

import seaborn as sns

tips = sns.load_dataset('tips')

g = sns.FacetGrid(tips, col='time', row='sex')
g.map(sns.scatterplot, 'total_bill', 'tip')
g.tick_params(direction='in', length=6, width=2) # on both
g.tick_params(colors='blue', axis='x')
g.tick_params(colors='green', axis='y')

Screen Shot 2022-08-07 at 10 51 44 AM

import seaborn as sns

penguins = sns.load_dataset('penguins')

g = sns.PairGrid(penguins, diag_sharey=False, corner=True)
g.map_lower(sns.scatterplot)
g.map_diag(sns.kdeplot)
g.tick_params(direction='in', length=6, width=2, color='blue')

Screen Shot 2022-08-06 at 8 47 55 PM

@stefmolin
Copy link
Contributor Author

I wasn't sure if you wanted to put this in the 0.12.0 release or the 0.13.0 release, so I held off on adding to the What's New for now. I also didn't add to the examples in case you wanted to either leave it out or had a specific change in mind. Let me know what you're thinking.

@codecov
Copy link

codecov bot commented Aug 7, 2022

Codecov Report

Merging #2944 (a3680e8) into master (9771eae) will increase coverage by 0.00%.
The diff coverage is 100.00%.

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2944   +/-   ##
=======================================
  Coverage   98.30%   98.31%           
=======================================
  Files          69       69           
  Lines       22985    23030   +45     
=======================================
+ Hits        22596    22641   +45     
  Misses        389      389           
Impacted Files Coverage Δ
seaborn/axisgrid.py 97.20% <100.00%> (+0.01%) ⬆️
tests/test_axisgrid.py 99.38% <100.00%> (+0.01%) ⬆️
seaborn/_statistics.py 100.00% <0.00%> (ø)
tests/test_statistics.py 100.00% <0.00%> (ø)
seaborn/_core/properties.py 99.15% <0.00%> (ø)
seaborn/_marks/scatter.py
tests/_marks/test_scatter.py
seaborn/_marks/dot.py 100.00% <0.00%> (ø)
tests/_marks/test_dot.py 100.00% <0.00%> (ø)

Copy link
Owner

@mwaskom mwaskom left a comment

Choose a reason for hiding this comment

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

Thanks @stefmolin, looks great, just a couple tiny comments.

seaborn/axisgrid.py Outdated Show resolved Hide resolved
seaborn/axisgrid.py Outdated Show resolved Hide resolved
tests/test_axisgrid.py Outdated Show resolved Hide resolved
tests/test_axisgrid.py Outdated Show resolved Hide resolved
@mwaskom
Copy link
Owner

mwaskom commented Aug 7, 2022

I wasn't sure if you wanted to put this in the 0.12.0 release or the 0.13.0 release, so I held off on adding to the What's New for now.

This can go into 0.12 as I haven't cut a release candidate yet, so please update the release notes.

@mwaskom mwaskom added this to the v0.12.0 milestone Aug 7, 2022
@stefmolin
Copy link
Contributor Author

Comments addressed and release notes updated. I also updated the usage of ravel() in the tests from my previous PR for refline().

@stefmolin stefmolin requested a review from mwaskom August 9, 2022 01:11
Copy link
Owner

@mwaskom mwaskom left a comment

Choose a reason for hiding this comment

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

Brilliant, thanks @stefmolin! Last bit, and sorry for missing this in my first review, but we should avoid accessing private matplotlib attributes as a general rule. I think the suggested change will work, and then this is 🟢🟢🟢

tests/test_axisgrid.py Outdated Show resolved Hide resolved
tests/test_axisgrid.py Outdated Show resolved Hide resolved
@mwaskom
Copy link
Owner

mwaskom commented Aug 9, 2022

Odd, I wonder why that didn't work.

Since matplotlib seems to be doing something weird here, probably best to use a different property. pad seems to work:

ax = plt.gca()
ax.tick_params(pad=4.7)
yt, *_ = ax.yaxis.get_major_ticks()
print(yt.get_pad())

4.7

Feel free to change locally and force push over my attempt at a fix.

@stefmolin stefmolin force-pushed the feature/grid_tick_params branch from 09cd646 to e4bd180 Compare August 10, 2022 00:46
@stefmolin
Copy link
Contributor Author

Switched to pad, as suggested.

@stefmolin stefmolin requested a review from mwaskom August 10, 2022 00:48
@mwaskom mwaskom merged commit 72d1322 into mwaskom:master Aug 10, 2022
@mwaskom
Copy link
Owner

mwaskom commented Aug 10, 2022

Wonderful!

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

Successfully merging this pull request may close these issues.

Add FacetGrid-level tick_params method
2 participants