Skip to content

Webapp data and scripts#77

Open
jaceybronte wants to merge 3 commits into
WayScience:mainfrom
jaceybronte:web-app
Open

Webapp data and scripts#77
jaceybronte wants to merge 3 commits into
WayScience:mainfrom
jaceybronte:web-app

Conversation

@jaceybronte

Copy link
Copy Markdown
Member

This has all of the data that should be necessary for the webapp, as well as a few shiny PCAs and the code for that.

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@MikeLippincott MikeLippincott self-requested a review June 8, 2026 21:17

@MikeLippincott MikeLippincott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Ready to merge after addressing comments!

Comment on lines +13 to +15
script_directory = pathlib.Path("../utils/").resolve()
sys.path.insert(0, str(script_directory))
from data_loader import load_model_data, load_train_test_data

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pointing out package anti-pattern here

Comment on lines +61 to +68

features = combined_df[gene_cols]


# In[8]:


features

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider merging these two into one cell and calling "head"

Comment on lines +74 to +75
import random
import colorsys

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

imports should be at top

Comment on lines +77 to +90
def generate_random_palette(num_colors, seed=12):
# Generate random colors with varied lightness and saturation
random.seed(seed)

colors = []

for _ in range(num_colors):
h = random.random() # Random hue between 0 and 1
l = random.uniform(0.2, 0.8) # Lightness between 0.3 and 0.9 for contrast
s = random.uniform(0.5, 1.0) # Saturation between 0.6 and 1.0 for vivid colors
color = colorsys.hls_to_rgb(h, l, s) # Convert HLS to RGB color
colors.append(color)

return colors

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Helper function might live better in the utils file, if not then consider moving to the top.

Add docstrings and type hints also

Comment on lines +96 to +99
import plotly.graph_objects as go
import plotly.express as px
from sklearn.decomposition import PCA
from sklearn.preprocessing import StandardScaler

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

imports at top



# Step 2: Subset based on matching keys
subset_keys = ["model", "latent_dim_total", "init", "z"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not used

# In[18]:


def generate_random_palette(num_colors, seed=12):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

consider moving to top, adding type hints and adding docstrings

# In[19]:


def make_dropdown_pca_with_selection(df, title="PCA Interactive Plot"):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here

out = widgets.Output() # Create a fresh output for this plot!

# Prepare PCA input
pca_input = df.drop(columns=["OncotreePrimaryDisease", "source", "ModelID"], errors="ignore").fillna(0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fillna with - might not be the correct strategy here? TBD

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.

2 participants