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

Update VolcanoPlot props #554

Merged
merged 25 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0aec15d
Update volcanoplot args
HammadTheOne Apr 7, 2021
b2f6506
Update bundle
HammadTheOne Apr 7, 2021
712dbaa
Remove default legend
HammadTheOne Apr 7, 2021
52f05e5
Merge branch 'master' into update-volcano
HammadTheOne Apr 7, 2021
bae0210
Adding x_axis and y_axis props
HammadTheOne Apr 11, 2021
e36d4b6
Added percy snapshot for layout props
HammadTheOne Apr 11, 2021
0779b53
Merge branch 'update-volcano' of https://github.com/plotly/dash-bio i…
HammadTheOne Apr 11, 2021
fef41e4
Added convert canvas arg
HammadTheOne Apr 11, 2021
16bc23f
Temporarily remove snapshot
HammadTheOne Apr 11, 2021
6f05a3b
Added default Title and Legend
HammadTheOne Apr 13, 2021
c0ed009
Linting fix
HammadTheOne Apr 13, 2021
e83f408
Another linting fix
HammadTheOne Apr 13, 2021
2d81982
Merge branch 'master' into update-volcano
HammadTheOne Apr 14, 2021
4640a15
Enable percy snapshot
HammadTheOne Apr 14, 2021
134ffff
Updating snapshot
HammadTheOne Apr 14, 2021
97e25da
Linting
HammadTheOne Apr 14, 2021
96bd56d
Changing variable name
HammadTheOne Apr 14, 2021
8fba9b2
Testing alternative snapshot
HammadTheOne Apr 14, 2021
a5b996a
Remove unnecessary args
HammadTheOne Apr 14, 2021
990217e
Update test params
HammadTheOne Apr 14, 2021
1e7bac2
Fixed bundle.map dynamic
HammadTheOne Apr 14, 2021
979df6b
Move defaults to layout
HammadTheOne Apr 15, 2021
e3dfcb4
Moved suggested keys in docstring
HammadTheOne Apr 15, 2021
56948c3
Removing update_axes
HammadTheOne Apr 15, 2021
3e2ce66
Removed explicit title arg
HammadTheOne Apr 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* [#543](https://github.com/plotly/dash-bio/pull/543) Added Dash Pileup component.
* [#547](https://github.com/plotly/dash-bio/pull/547) Added shapes and isosurfaces props to 3dMoleculeViewer to enable rendering additional features on the molecule.
* [#553](https://github.com/plotly/dash-bio/pull/553) Added source mapping.
* [#554](https://github.com/plotly/dash-bio/pull/554) Added additional props and arbitrary layout arguments to VolcanoPlot.


## [0.6.1] - 2021-02-15
Expand Down
2 changes: 1 addition & 1 deletion R/internal.R
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ version = "0.6.2", src = list(href = NULL,
file = "deps"), meta = NULL,
script = 'bundle.js.map',
stylesheet = NULL, head = NULL, attachment = NULL, package = "dashBio",
all_files = FALSE), class = "html_dependency"),
all_files = FALSE, dynamic = TRUE), class = "html_dependency"),
`dash_bio-shared` = structure(list(name = "dash_bio-shared",
version = "0.6.2", src = list(href = NULL,
file = "deps"), meta = NULL,
Expand Down
3 changes: 2 additions & 1 deletion dash_bio/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@
'https://unpkg.com/dash-bio@{}'
'/' + package_name + '/bundle.js.map'
).format(__version__),
'namespace': package_name
'namespace': package_name,
'dynamic': True
}
])

Expand Down
2 changes: 1 addition & 1 deletion dash_bio/bundle.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dash_bio/bundle.js.map

Large diffs are not rendered by default.

77 changes: 59 additions & 18 deletions dash_bio/component_factory/_volcano.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ def VolcanoPlot(
gene='GENE',
annotation=None,
logp=True,
title='Volcano Plot',
xlabel=None,
ylabel='-log10(p)',
point_size=5,
Expand All @@ -33,7 +32,8 @@ def VolcanoPlot(
genomewideline_color='grey',
genomewideline_width=1,
highlight=True,
highlight_color="red"
highlight_color="red",
**kwargs
):
"""Return a Dash Bio VolcanoPlot figure.

Expand Down Expand Up @@ -72,7 +72,6 @@ def VolcanoPlot(
plotting the raw value could be useful for other genome-wide plots
(e.g., peak heights, Bayes factors, test statistics, and other
"scores").
- title (string; default 'Volcano Plot'): Title of the graph.
- xlabel (string; optional): Label of the x axis.
- ylabel (string; default '-log10(p)'): Label of the y axis.
- point_size (number; default 5): Size of the points of the Scatter
Expand Down Expand Up @@ -110,11 +109,29 @@ def VolcanoPlot(
dataframe = pd.DataFrame(
np.random.randint(0,100,size=(100, 2)),
columns=['P', 'EFFECTSIZE'])
fig = create_volcano(dataframe, title='XYZ Volcano plot')
fig = create_volcano(dataframe, title=dict(text='XYZ Volcano plot'))

plotly.offline.plot(fig, image='png')
'''

- Additional keys (misc.): Arbitrary arguments can be passed to modify the
Layout and styling of the graph. A full reference of acceptable args is
available [here](https://plotly.com/python-api-reference/generated/plotly.graph_objects
.Layout.html).

Some commonly used layout keys are:
- title (dict: optional): Dict with compatible properties for the title of
the figure layout.
- xaxis (dict: optional): Dict with compatible properties for the x-axis of
the figure layout.
- yaxis (dict: optional): Dict with compatible properties for the y-axis of
the figure layout.
- height (number; optional): Sets the plot's height (in px).
- width (number; optional): Sets the plot's width (in px).
- margin (dict | plotly.graph_objects.layout.Margin instance): A dict or Margin
instance that sets the separation between the main plotting space and
the outside of the figure.
- legend (dict | plotly.graph_objects.layout.Legend instance): A dict or Legend
instance with compatible properties.
"""

vp = _VolcanoPlot(
Expand All @@ -128,7 +145,6 @@ def VolcanoPlot(
)

return vp.figure(
title=title,
xlabel=xlabel,
ylabel=ylabel,
point_size=point_size,
Expand All @@ -140,7 +156,8 @@ def VolcanoPlot(
genomewideline_color=genomewideline_color,
genomewideline_width=genomewideline_width,
highlight=highlight,
highlight_color=highlight_color
highlight_color=highlight_color,
**kwargs
)


Expand Down Expand Up @@ -274,7 +291,6 @@ def __init__(

def figure(
self,
title='Volcano Plot',
xlabel=None,
ylabel='-log10(p)',
point_size=5,
Expand All @@ -287,14 +303,13 @@ def figure(
genomewideline_width=1,
highlight=True,
highlight_color='red',
**kwargs
):
"""Return a figure object compatible with plotly.graph_objects.

Keyword arguments:
- title (string; default 'Volcano Plot'): Title of the
graph.
- xlabel (string; optional): Label of the x axis.
- ylabel (string; default '-log10(p)'): Label of the y axis.
- xlabel (string; optional): Label of the x-axis.
- ylabel (string; default '-log10(p)'): Label of the y-axis.
- point_size (number; default 5): Size of the points of the scatter
plot.
- col (string; optional): Color of the points of the Scatter plot. Can
Expand Down Expand Up @@ -322,6 +337,25 @@ def figure(
- highlight_color (string; default 'red'): Color of the data points
highlighted because considered significant. Can be in any color
format accepted by plotly.graph_objects.
- Additional keys (misc.): Arbitrary arguments can be passed to modify the
Layout and styling of the graph. A full reference of acceptable args is
available [here](https://plotly.com/python-api-reference/generated/plotly.graph_objects
.Layout.html).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're accepting **kwargs here and passing them to go.Layout, do we actually need any of the other args you're adding here? We can list some of the common ones (including the ones you're including explicitly now) in the description here, they'd all work identically as part of kwargs.

x_axis and y_axis: in the docstring there's no _ and I'd argue it's cleaner without, then these can just be left to kwargs as well.

In order to keep the defaults you have but allow users to update the layout arbitrarily with kwargs after that, I suspect the easiest would be to generate the default layout first, then do layout.update(**kwargs). A little exploring shows that this does a deep merge, so if your kwargs are just xaxis=dict(color="red") you'll still get the rest of the x axis as set in the default layout.

Copy link
Contributor Author

@HammadTheOne HammadTheOne Apr 14, 2021

Choose a reason for hiding this comment

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

do we actually need any of the other args you're adding here?

I thought it might be nice to have the most commonly used args available explicitly, especially for users that use IDE's which show parameter info, but simply including them as part of the documentation is also an option. However, retaining them as explicit args would also potentially save users a trip to the Plotly graphing library docs (unless we also update dash-docs with some better examples). I think it's a trade-off between cleaner params (with everything as kwargs) and easier "default" use of the component.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair point re: IDE docs & completion. That said for everything except height and width you're still going to need to visit the graphing library docs to find out what goes inside the object, and it feels a bit arbitrary to be pulling in just a few layout options - as if the ones in kwargs are less-fully-supported or something. So I still think we'll be better off removing all of them and just documenting a few (the set you've proposed adding here looks good) in the docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're still going to need to visit the graphing library docs to find out what goes inside the object,

That's a great point - Updated in a5b996a.


Some commonly used layout keys are:
- title (dict: optional): Dict with compatible properties for the title of
the figure layout.
- xaxis (dict: optional): Dict with compatible properties for the x-axis of
the figure layout.
- yaxis (dict: optional): Dict with compatible properties for the y-axis of
the figure layout.
- height (number; optional): Sets the plot's height (in px).
- width (number; optional): Sets the plot's width (in px).
- margin (dict | plotly.graph_objects.layout.Margin instance): A dict or Margin
instance that sets the separation between the main plotting space and
the outside of the figure.
- legend (dict | plotly.graph_objects.layout.Legend instance): A dict or Legend
instance with compatible properties.
"""

if xlabel is None:
Expand Down Expand Up @@ -355,13 +389,18 @@ def figure(
col = 'black'

layout = go.Layout(
title=title,
title={'text': 'Volcano Plot',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we be using the title arg here? As it is we're ignoring it, right? Though passing the title arg directly to title.text in the layout means you can't use the dict form of title to modify its styling. Then the only way to do that would be to use magic underscores in the kwargs (title_font_color="red" etc).

Alternatively we could drop title as an arg completely, so it gets picked up by layout.update. The downside of that approach is that passing in a string for title would wipe out the default style you have below - at least until plotly/plotly.py#3148 gets fixed, which hopefully will not be too long (🙏 @nicolaskruchten )

So yeah, I guess my inclination is to drop title as an arg, even though it's a breaking change in the short term, because the current behavior will be restored with much more flexibility after we fix plotly/plotly.py#3148

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped in 3e2ce66.

'font': {'family': 'sans-serif', 'size': 20},
'x': 0.5,
'xanchor': 'right',
'yanchor': 'top'
},
hovermode='closest',
legend={
'x': 0.85,
'y': 0.1,
'bgcolor': '#f2f5fa'
},
legend={'bgcolor': '#ebf1fa',
'yanchor': 'top',
'x': 1.01,
"font": {"family": "sans-serif"}
},
xaxis={
'title': xlabel,
'zeroline': False,
Expand All @@ -373,6 +412,8 @@ def figure(
}
)

layout.update(**kwargs)

data_to_plot = [] # To contain the data traces
tmp = pd.DataFrame() # Empty DataFrame to contain the highlighted data

Expand Down
2 changes: 1 addition & 1 deletion inst/deps/bundle.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion inst/deps/bundle.js.map

Large diffs are not rendered by default.

38 changes: 36 additions & 2 deletions tests/integration/test_volcano_plot.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import json
import pandas

from common_features import nested_component_layout, \
nested_component_app_callback

import dash
import dash_html_components as html
import dash_bio

from common_features import nested_component_layout, \
nested_component_app_callback

_data = None

Expand Down Expand Up @@ -129,3 +130,36 @@ def test_dbvp005_effect_size_line_value(dash_duo):
data_prop_name='dataframe',
take_snapshot=True
)


def test_dbvp006_test_layout_props(dash_duo):
app = dash.Dash(__name__)

app.layout = html.Div(nested_component_layout(
dash_bio.VolcanoPlot(
dataframe=_data,
width=600,
legend={
'x': 0.85,
'orientation': 'h',
'yanchor': 'bottom',
'y': 1.02,
'bgcolor': '#f2f5fa'
},
xaxis={"color": "red"},
yaxis={"color": "blue"},
template="simple_white",
)
))

nested_component_app_callback(
app,
dash_duo,
component=dash_bio.VolcanoPlot,
component_data=_data,
test_prop_name='plot_bgcolor',
test_prop_value='pink',
prop_value_type='string',
data_prop_name='dataframe',
take_snapshot=True
)