-
Notifications
You must be signed in to change notification settings - Fork 362
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
Extend color legend size for scatter charts for matplotlib #286
Conversation
Thanks @caitlynachen! This looks like a good start. Here are a couple of cases where the figure size doesn't quite work out. The plot figure looks overly stretched out. We might also need to abbreviate the legend when the number of colors is too large. |
fixed corner cases @dorisjlee mentioned |
Thanks @caitlynachen! A few more comments on the latest changes:
|
for i in range(len(unique)): | ||
unique[i] = str(unique[i]) | ||
if len(unique[i]) > 26: | ||
unique[i] = unique[i][:26] + "..." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not showing up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the abbreviation "..." is not showing up in the legend, see comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you seeing something differently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm able to see it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh this is a good point, a user might be confused why a specific category is not showing up, so something in the legend would be nice to address this. Another idea rather than using the '...' (which seems a bit off to me) could be to use multiple columns. Try maybe something like this: https://stackoverflow.com/questions/58344791/how-to-handle-categorical-legends-with-too-many-entries-to-match-a-fixed-chart-h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh the "..." is not for the length of the legend, but rather the individual categories. I checked and Altair also didn't have a "..." at the end of the list, so didn't include it in this PR. If you want, maybe I can tackle that issue separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an issue #295 for this new change
for i in range(len(unique)): | ||
unique[i] = str(unique[i]) | ||
if len(unique[i]) > 26: | ||
unique[i] = unique[i][:26] + "..." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm able to see it.
Hi @caitlynachen, Thanks for adding in #295. Could you look into addressing the earlier comment on consistency across fontsizes when the image stretching happens? |
Added fontsize to xlabel, ylabel and legend attributes to maintain fontsize to be as large as possible within the range of plt.tight_layout |
In response to @dorisjlee 's comment about reduced font size on these scatter charts with color legends, I tried a number of solutions:
Despite the changes, the font size still remained smaller than normal. Not entirely sure why this is the case, especially since when I print out Resources: |
Thanks for the explanation @caitlynachen! I'm merging this in for now, for the matplotlib font size issue, could you create a separate issue outlining this problem? I believe this is a more general issue with the font size and figures for matplotlib and is not particular to just the axis label (affects all text and elements on the figure when the figure is resized). |
Changed figsize for scatter charts
![Screen Shot 2021-02-23 at 5 53 15 PM](https://user-images.githubusercontent.com/11529801/108934240-6830b700-7601-11eb-8f0d-6eda83dda4cf.png)
![Screen Shot 2021-03-07 at 9 04 17 PM](https://user-images.githubusercontent.com/11529801/110276748-b0988f00-7f88-11eb-9935-291f0733de67.png)