Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Opacity feature #22

Merged
merged 4 commits into from
Mar 25, 2019
Merged

Conversation

ismailuddin
Copy link
Contributor

Added ability to change opacity for markers in functions:

  • px.scatter()
  • px.scatter_3d()
  • px.scatter_matrix()
  • px.scatter_ternary()
  • px.scatter_polar()

Usage

px.scatter(df, x='column1', y='column2', opacity=0.5)

Simply add the opacity argument to any of the functions, and specific a float between 0 and 1.

@gepcel
Copy link

gepcel commented Mar 22, 2019

There's alpha everywhere in matplotlib, is it possible to support alpha as an alias? .It's ok if it's not. I understand it's too more to ask.

@ismailuddin
Copy link
Contributor Author

It's just because the way opacity is specified in plotly graphs is different for every type of plot, unlike maptlotlib where its usually a top level argument in the main function. So for the other types of graphs, I need to check specifically how plotly handles it for those graphs. I'll try to look into it when I have time.

@gepcel
Copy link

gepcel commented Mar 22, 2019

It's just matplotlib uses alpha in python, and ggplot uses alpha in R. So if plotly_express can support alpha as an alias name of opacity, it would be more easy for people not familiar with plotly.

@ismailuddin
Copy link
Contributor Author

Oh sorry I misunderstood what you meant originally, yes I guess it could be done that way, but that's probably up to the plotly_express / plotly developers to decide. Probably have to keep opacity for sake of consistency...

@nicolaskruchten
Copy link
Contributor

Thanks for this PR! Our first contribution to the source :)

I do favour opacity instead of alpha as that's what it's called in the underlying tooling. alpha is very technical and not at all descriptive of the visual variable being controlled :)

A couple of requests:

  1. Can you add a docstring in plotly_express/_doc.py please?
  2. It would be best if you could set this in the infer_config() function as it's the same value for all points (the make_trace_kwargs() function is intended for attributes which vary from point to point)... this isn't at all clear from the code today and for this I apologize: I'll try to rename the functions and/or add some signpost comments to guide future PRs!

@ismailuddin
Copy link
Contributor Author

You're welcome! I'm really glad this package got made, I was seriously thinking of making something similar myself the other day and now its already been done!!

I've moved the code as requested, and added the docstring. I had an error generating the HTML docs (ValueError: I/O operation on closed file) even prior to any modification, which I haven't quite figured out why though...

@nicolaskruchten
Copy link
Contributor

Yep, pretty simple in the end :)

@@ -91,6 +91,10 @@
"Ignored if `error_z` is `None`.",
],
color=[colref, "Values from this column are used to assign color to marks."],
opacity=[
"Sets the opacity of the markers in scatter plots such as `scatter()`",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll tweak this wording a bit. To match the rest it should be something like (number between 0 and 1) Sets the opacity of markers. (no need to enumerate the function types as this shows up in the individual doc strings of those functions.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've adjusted to keep it more inline, feel free to change as necessary!

@nicolaskruchten
Copy link
Contributor

I'm happy to merge this in a few hours when I've had a chance to test it a bit. Thanks again for the contribution and feel free to suggest additional ones now that you've seen the pattern :)

@nicolaskruchten nicolaskruchten merged commit 8cda34b into plotly:master Mar 25, 2019
@nicolaskruchten
Copy link
Contributor

Thanks again!

@DmitriyG228
Copy link

Hi, scatter_mapbox and scatter_geo also seen to miss opacity feature

@nicolaskruchten
Copy link
Contributor

@DmitriyG228 thanks for letting me know! I'll add this to the next release, which should come out within the hour :)

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

Successfully merging this pull request may close these issues.

4 participants