Skip to content

Conversation

@LizaShch
Copy link
Collaborator

@LizaShch LizaShch commented Apr 3, 2025

Created datashader heatmap function with faceted plots based off annotations as well as included test cases for usability.

@LizaShch LizaShch requested a review from ThomasSheeley April 3, 2025 20:30
@LizaShch LizaShch requested a review from Ahmad8864 April 9, 2025 22:21
def heatmap_datashader(x, y, labels=None, theme=None,
x_axis_title="Component 1", y_axis_title="Component 2",
plot_title=None, **kwargs):
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure the docstring follows the same format as the other functions in visualizations. Also make sure all your lines are < 80 chars in length

Parameters
----------
var_name: type, [optional/default]
    description

Returns
-------
...

logging.basicConfig(level=logging.INFO,
format='%(asctime)s - %(levelname)s - %(message)s')

def heatmap_datashader(x, y, labels=None, theme=None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the input x and y, and not an adata object? I think all the spac functions are supposed to have adata as input

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a low-level function which is designed to only accept x and y coordinates. See visualize_2d_scatter() as an example which does not accept adata but rather x and y.

'inferno': ds.colors.inferno,
'color_lookup': ds.colors.color_lookup,
}
cmap = themes.get(theme, ds.colors.viridis) # Default to 'viridis' if theme is not specified
Copy link
Collaborator

Choose a reason for hiding this comment

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

George wants us to use the get_defined_color_map function to get the color_map from the adata object for consistency across functions

heatmap_datashader(self.x, self.y, labels=wrong_labels)
self.assertIn("Labels length should match x and y length", str(context_manager.exception))


Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a test of it running with valid inputs to make sure it returns a figure and correct number of subplots

Copy link
Collaborator

@Ahmad8864 Ahmad8864 left a comment

Choose a reason for hiding this comment

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

LGTM

'inferno': ds.colors.inferno,
'color_lookup': ds.colors.color_lookup,
}
cmap = themes.get(theme, ds.colors.viridis) # Default to 'viridis' if theme is not specified
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this comment to its own line so the line is >80 chars

LizaShch and others added 30 commits April 17, 2025 20:38
fix(select_values): added support when observation are numerical
…dataset

fix(summarize_dataframe): remove duplicated missing index
…ncolor

feat(visualize_nearest_neighbor): add pin-color and corresponding uni…
merge updated summarize_dataframe and visualize_nearest_neighbor with pin-color from dev to release_dev
…and helpers and unit test files individually
…nearest_neighbor

feat(pin_color): add pin color feature to visualize nearest neighbor …
merge dev into release_dev with updated visualize_nearest_neighbor by adding pin-color feature
…nearest_neighbor

fix(visualize_nearest_neighbor): rewrite unit tests and move up  libr…
merge the updated visualize_nearest_neighbor function and unit tests from dev to dev_release
step: bumping minor version from 0.8 to 0.9
step: bumping minor version from 0.8 to 0.9
Minor version bumping from v0.8.0 to v0.9.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.