-
Notifications
You must be signed in to change notification settings - Fork 11
Datashader Heatmap Visualization Function #330
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
base: dev
Are you sure you want to change the base?
Conversation
Add layer information in histogram title
…he import of unneccesary libraries
georgezakinih
left a comment
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.
@LizaShch @ThomasSheeley , Thanks for the PR. I left few comments.
src/spac/visualization.py
Outdated
|
|
||
| for i, cat in enumerate(categories): | ||
| subset = coords[coords["labels"] == cat] | ||
| canvas = ds.Canvas( |
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.
@LizaShch, consider combining this the code at line 133 using the "partial" construct in python. This way, when you edit it, it will be edited once.
| # Fixed categorical labels to ensure representation of each category | ||
| fixed_labels = ['A', 'B', 'C', 'A', 'B', 'C', 'A', 'B', 'C', 'A'] | ||
| self.labels_categorical = pd.Series(fixed_labels, dtype="category") | ||
| self.labels_continuous = pd.Series(np.random.rand(10)) |
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.
@LizaShch , what do you do with teh continuous labels?
| heatmap_datashader(self.x, self.y, labels=wrong_labels) | ||
| self.assertIn("Labels length should match x and y length", str(context_manager.exception)) | ||
|
|
||
| def test_valid_input_returns_figure(self): |
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.
@LizaShch, would you please add a case with number of labels not multiple of 3?
| # There should be as many subplots as unique labels (3 in this case) | ||
| num_axes = len(fig.axes) | ||
| expected_axes = self.labels_categorical.nunique() | ||
| self.assertEqual(num_axes, expected_axes) |
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.
@LizaShch, any other assertion you can add to make sure the plot got generated correctly besides the number of axes? For example, the content of the plot?
|
|
||
| def heatmap_datashader(x, y, labels=None, theme=None, | ||
| x_axis_title="Component 1", y_axis_title="Component 2", | ||
| plot_title=None, **kwargs): |
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.
@LizaShch, can we use kwargs to pass optional arguments to ds.Canvas to control the figure? This way, you don't need to explicty control only for plot width and plot height, but for all other input argument that function would tak.
ying39purdue
left a comment
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.
@georgezakinih I added a few formatting additions and Liza's previous commit resolved the previous comments
Summary:
This pull request includes changes to src/spac/visualization.py file to creating a new heatmap function using the package datashader. There is an option to facet plots by annotation as well. A rest file tests/test_visualization/test_datashader.py was added as well to ensure proper functionality.
Changes: