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.

Sign up for GitHub

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

Jump to bottom

Decreased legend visibility on massive-plots #66

Closed
quantum9Innovation opened this issue Jun 17, 2021 · 5 comments
Closed

Decreased legend visibility on massive-plots #66

quantum9Innovation opened this issue Jun 17, 2021 · 5 comments
Labels
bug ⚠️ Something isn't working good first issue 👍 Good for newcomers low-priority 🔽 Will be worked on later nightly 🌙 Nightly builds or releases src Changes to the package source code
Projects
Management v2

Comments

@quantum9Innovation
Copy link
Member

quantum9Innovation commented Jun 17, 2021 •
edited by Quantalabs

Describe the bug
The newly-added support for native stacked area charts in massive-plots (epi.plots.native.stacked()) lacks good legend visibility. This is due to the fact that for some reason, matplotlib fails to enforce the default white background for graphical elements on plots, resulting in no contrast between the graph itself and the legend.

To Reproduce
See tests/plotting::test_full_native_stack

Expected behavior
All legends should have a white background, thus standing out from the rest of the plot.

Screenshots
epispot-issue-screenshot-3
As shown in the plot above, legend colors blend in with the actual graphics.

Python

  • Python Version: 3.9.4
  • Operating System: Linux Mint 20 (Cinnamon)
  • Epispot Version: massive-plots @ 4068117
  • Nightly (yes/no): yes: pre-alpha

Anaconda

  • Anaconda Version: 4.10.1
  • Matplotlib Version: 3.3.4

Additional context
This issue was mentioned in commit 4068117
@quantum9Innovation quantum9Innovation added bug ⚠️ Something isn't working good first issue 👍 Good for newcomers nightly 🌙 Nightly builds or releases low-priority 🔽 Will be worked on later src Changes to the package source code status:available labels Jun 17, 2021
@quantum9Innovation quantum9Innovation added this to Needs triage in Management v2 via automation Jun 17, 2021
@quantum9Innovation quantum9Innovation moved this from Needs triage to Low priority in Management v2 Jun 17, 2021
@quantum9Innovation
Copy link
Member Author

quantum9Innovation commented Jun 17, 2021 •
edited

Update

This will not be fixed in the upcoming PR merging massive-plots → master. We expect to close this issue with a separate patch PR that will be submitted before the mini-plots project is started. That being said, this issue is still available for claim (respond with "/claim" (without quotes) to claim this issue) and any help will be greatly appreciated.

@quantum9Innovation quantum9Innovation added the wontfix 🔨 This will not be worked on label Jun 17, 2021
@quantum9Innovation quantum9Innovation mentioned this issue Jun 17, 2021
Merged
@lisphilar
Copy link
Contributor

lisphilar commented Jun 18, 2021 •
edited

Dear @quantum9Innovation,
To place the legend outside of the plotting area may be an alternative if it will not lead any problems.

ax.legend(loc="lower center", bbox_to_anchor=(0.5, -0.2), borderaxespad=0, ncol=2)
plt.tight_layout()

quantum9Innovation added a commit that referenced this issue Jun 19, 2021
@quantum9Innovation @lisphilar
Fix #66 via suggestion from @lisphilar
0ca85d1 
This commit fixes "Decreased legend visibility on `massive-plots`" (#66)
 via a suggestion by @lisphilar.

 Changelist:
 - Fix `epi.plots.native::stacked`

Co-authored-by: Lisphilar <7270139+lisphilar@users.noreply.github.com>
@quantum9Innovation
Copy link
Member Author

quantum9Innovation commented Jun 19, 2021 •
edited

@lisphilar Thank you for the advice! I've added this fix to a new branch (patch-66) via commit 0ca85d1. I also added you as a co-author to the commit as: Lisphilar <7270139+lisphilar@users.noreply.github.com>

The plot now looks like this:
patch_66

@quantum9Innovation
Copy link
Member Author

I'll close the issue as soon as this branch is merged.

@quantum9Innovation quantum9Innovation added status:claimed and removed status:available wontfix 🔨 This will not be worked on labels Jun 19, 2021
@quantum9Innovation quantum9Innovation mentioned this issue Jun 19, 2021
Merged
quantum9Innovation added a commit that referenced this issue Jun 19, 2021
@quantum9Innovation
Merge pull request #70 from epispot/patch-66
dfa4d6a 
Fix #66
Management v2 automation moved this from Low priority to Closed Jun 19, 2021
@lisphilar
Copy link
Contributor

Thank you for updating and mentioning me at the commit!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug ⚠️ Something isn't working good first issue 👍 Good for newcomers low-priority 🔽 Will be worked on later nightly 🌙 Nightly builds or releases src Changes to the package source code
Projects
No open projects
Management v2
  
Closed
Milestone
No milestone
Development

No branches or pull requests
3 participants
@lisphilar @quantum9Innovation @Quantalabs