Add Sankey diagram visualization functions #989
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
for more information, see https://pre-commit.ci
Zethson
left a comment
Zethson
left a comment
There was a problem hiding this comment.
Thank you very much!
- Don't forget add the function to our docs, please.
- Could you please add previews to the PR description?
- We should have a consistent plotting user interface where all have the same parameters and style like height, width etc. We should avoid kwargs where possible.
- Great tests! We might need to use try -> finally because you're changing the plotting backend. If the test fails and the plotting backend is not reset, it could cause other tests to fail.
| edata: EHRData, | ||
| *, | ||
| columns: list[str], | ||
| show: bool = False, |
There was a problem hiding this comment.
I am not using a show parameter for holoviews plots anymore because I think they're shown by default. Do we really still need this?
| *, | ||
| columns: list[str], | ||
| show: bool = False, | ||
| **kwargs, |
There was a problem hiding this comment.
Could you please look at the parameters of the survival analysis plots? (I'll make another PR very soon but one of them is already updated). We should have parameters like height, width etc. Consistency is very very important.
| ) -> hv.Sankey: | ||
| """Create a Sankey diagram showing relationships across observation columns. | ||
| Please call :func:`holoviews.extension` with ``"matplotlib"`` or ``"bokeh"`` before using this function to select the backend. |
There was a problem hiding this comment.
| Please call :func:`holoviews.extension` with ``"matplotlib"`` or ``"bokeh"`` before using this function to select the backend. |
I think we should set a default backend and these functions will error if none is set.
| >>> import pandas as pd | ||
| >>> import ehrdata as ed | ||
| >>> | ||
| >>> layer = np.array( |
There was a problem hiding this comment.
This example is too complex. Can we make this work with blobs? If not, can we adapt the blobs function so that you can use it here, please?
|
|
||
| default_opts = {"label_position": "right", "show_values": True, "title": f"Patient flows: {columns[0]} over time"} | ||
|
|
||
| default_opts.update(kwargs) |
There was a problem hiding this comment.
Worried about this - see above.
|
|
||
|
|
||
| @pytest.fixture | ||
| def ehr_3d_mini(): |
There was a problem hiding this comment.
Ideally with blobs as well - see above.
fixes #232
two new visualization functions plot_sankey and plot_sankey_time for creating Sankey diagrams to analyze patient flows and state transitions in EHR data, added comprehensive tests
plot_sankey : relationships across observation columns
plot_sankey_time : patient state transitions over time