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: Enable xticklabels for all bottom axes #3600

Merged
merged 4 commits into from
Dec 28, 2023

Conversation

MaozGelbart
Copy link
Contributor

Prior to this change, wrapped faceted plots (with shared axis) are created by deletion of some axes from a larger rectangular grid, so axes that are not on the bottom row lose their tick labels. Current code makes sure that all xtick labels are visible, but doesn't ensure that they're on. The proposed PR makes sure all "bottom" axes have their xticklabels on.
Fixes #3548 :
Screenshot 2023-12-22 at 12 43 57

@MaozGelbart MaozGelbart changed the title Enable xticklabels for all bottom axes FIX: Enable xticklabels for all bottom axes Dec 22, 2023
Copy link

codecov bot commented Dec 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b49e595) 98.52% compared to head (167fffa) 98.52%.
Report is 5 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3600   +/-   ##
=======================================
  Coverage   98.52%   98.52%           
=======================================
  Files          75       75           
  Lines       24701    24712   +11     
=======================================
+ Hits        24336    24348   +12     
+ Misses        365      364    -1     
Files Coverage Δ
seaborn/_core/plot.py 99.15% <100.00%> (+<0.01%) ⬆️
tests/_core/test_plot.py 98.84% <100.00%> (+<0.01%) ⬆️

... and 4 files with indirect coverage changes

if not _version_predates(mpl, "3.7"):
assert ax.xaxis.get_tick_params()["labelleft"]
else:
assert len(ax.get_xticklabels()) > 0
Copy link
Owner

Choose a reason for hiding this comment

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

I'm confused about how this is flagged as not tested since the pinned build log appears to have matplotlib 3.4 installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure about this comment by codecov, but its webpage for this test suite suggests it is tested (lines marked as green). See here.

Copy link
Owner

Choose a reason for hiding this comment

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

Agreed, the full code coverage report looks fine, not sure what's up with the bot.

@mwaskom
Copy link
Owner

mwaskom commented Dec 28, 2023

Thanks!

@mwaskom mwaskom merged commit 45a098f into mwaskom:master Dec 28, 2023
11 checks passed
@mwaskom mwaskom added this to the v0.13.1 milestone Dec 28, 2023
@MaozGelbart MaozGelbart deleted the fix_3548 branch December 28, 2023 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants