Skip to content

refactor styler and palettes#1

Merged
anmorgunov merged 7 commits intomasterfrom
ref/reorganize
Nov 3, 2025
Merged

refactor styler and palettes#1
anmorgunov merged 7 commits intomasterfrom
ref/reorganize

Conversation

@anmorgunov
Copy link
Contributor

@anmorgunov anmorgunov commented Nov 3, 2025

Greptile Overview

Updated On: 2025-11-03 19:33:48 UTC

Greptile Summary

This PR performs a major refactoring of the ischemist package structure and functionality. The changes reorganize the codebase from a nested src/ischemist/style/ structure to a flatter organization with core functionality moved to src/ischemist/colors.py and src/ischemist/plotly.py. The refactoring introduces a new immutable, theme-based styling system for Plotly figures and replaces the previous color handling with more robust, validated Color and ColorPalette classes.

Key structural changes include: moving the project from personal to organizational ownership (anmorgunov/ischemist → ischemist/py-style), creating a new dev subpackage for development utilities, and modernizing the CI/CD pipeline with new tooling (ty for type checking, uv for package management). The refactoring also introduces comprehensive testing for the color sampling algorithms and provides new developer utilities for palette generation and visualization.

Important Files Changed

Filename Score Overview
src/ischemist/colors.py 4/5 Complete rewrite introducing immutable Color and ColorPalette classes with validation and sampling
src/ischemist/plotly.py 4/5 New comprehensive styling system with theme support and immutable design patterns
src/ischemist/style/plotly.py 4/5 Deleted entire 351-line Styler class - functionality moved to new location
src/ischemist/style/colors.py 4/5 Deleted Color and ColorPalette classes - functionality moved to new colors.py
tests/test_colors.py 5/5 New comprehensive test suite for ColorPalette sampling with edge cases and validation
src/ischemist/dev/generate_palette.py 4/5 New palette generation utility using coloraide library and Oklab color space
src/ischemist/dev/visualize.py 4/5 New visualization utility for color palette grid display using Plotly
pyproject.toml 4/5 Updated project metadata, build system, and dependency management for organizational migration
.github/workflows/ci.yml 3/5 New CI workflow introducing modern tooling (ty, uv) replacing previous quality checks
.pre-commit-config.yaml 3/5 Refactored to use external ruff repos and replaced mypy with ty type checker
src/ischemist/dev/__init__.py 3/5 Moved from style subpackage and emptied - potential API breaking change
.github/workflows/quality.yml 3/5 Significantly simplified workflow removing dependency management and caching

Confidence score: 3/5

  • This PR requires careful review due to significant structural changes that could break existing APIs and imports
  • Score reflects concerns about potential breaking changes from module reorganization, incomplete documentation updates, and introduction of new dependencies that may not be familiar to all developers
  • Pay close attention to the CI workflow changes, module reorganization impacts on existing imports, and the new dependency on 'ty' and 'uv' tooling

Sequence Diagram

sequenceDiagram
    participant User
    participant Scripts as "Scripts (generate-palette.py, showcase-styler.py)"
    participant DevModule as "ischemist.dev"
    participant Colors as "ischemist.colors"
    participant Plotly as "ischemist.plotly"
    participant External as "External Libraries (coloraide, plotly, numpy)"

    User->>Scripts: "Run generate-palette.py"
    Scripts->>DevModule: "Import generate_palette, visualize"
    Scripts->>Colors: "Import ColorPalette"
    DevModule->>External: "Use coloraide for palette generation"
    DevModule-->>Scripts: "Return generated palettes"
    Colors->>Colors: "Create ColorPalette.from_hex_codes()"
    Scripts->>DevModule: "Call plot_palettes()"
    DevModule->>Plotly: "Use Styler for visualization"
    Plotly->>External: "Create plotly figures"
    External-->>User: "Display palette visualization"

    User->>Scripts: "Run showcase-styler.py"
    Scripts->>Colors: "Import PALETTES, ColorPalette"
    Scripts->>Plotly: "Import Styler"
    Scripts->>External: "Import numpy, plotly.graph_objects"
    Scripts->>DevModule: "Call plot_palettes()"
    Scripts->>Colors: "Call palette.sample()"
    Colors-->>Scripts: "Return sampled colors"
    Scripts->>Plotly: "Create Styler instance"
    Scripts->>External: "Create line plot with numpy data"
    Scripts->>Plotly: "Call styler.apply_style()"
    Plotly-->>Scripts: "Return styled figure"
    Scripts->>External: "Display figure"
    External-->>User: "Show styled visualizations"
Loading

@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ref/reorganize

Comment @coderabbitai help to get the list of available commands and usage tips.

@anmorgunov anmorgunov added scope/domain the core logic, abstractions, and internal workings of the package or application type/refactor internal code quality improvement. no user-facing changes labels Nov 3, 2025
@anmorgunov anmorgunov merged commit 2feaf6a into master Nov 3, 2025
3 checks passed
@anmorgunov anmorgunov deleted the ref/reorganize branch November 3, 2025 19:25
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

16 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

and aesthetically pleasing.

Usage:
- Run this script directly: `python -m ischemist.dev.palette_generator`
Copy link

Choose a reason for hiding this comment

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

syntax: The usage instructions reference python -m ischemist.dev.palette_generator but the module name is generate_palette

Suggested change
- Run this script directly: `python -m ischemist.dev.palette_generator`
- Run this script directly: `python -m ischemist.dev.generate_palette`
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ischemist/dev/generate_palette.py
Line: 16:16

Comment:
**syntax:** The usage instructions reference `python -m ischemist.dev.palette_generator` but the module name is `generate_palette`

```suggestion
  - Run this script directly: `python -m ischemist.dev.generate_palette`
```

How can I resolve this? If you propose a fix, please make it concise.

"""
# Get the original overrides used to create this styler
current_config_dict = dataclasses.asdict(self.config)
base_theme_dict = dataclasses.asdict(THEMES[self.config.name])
Copy link

Choose a reason for hiding this comment

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

logic: this line will raise KeyError if self.config.name doesn't exist in THEMES (e.g., with custom StyleConfig instances). Should with_theme handle cases where the current styler was created with a custom StyleConfig not in the theme registry?

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ischemist/plotly.py
Line: 171:171

Comment:
**logic:** this line will raise KeyError if self.config.name doesn't exist in THEMES (e.g., with custom StyleConfig instances). Should with_theme handle cases where the current styler was created with a custom StyleConfig not in the theme registry?

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +131 to +132
if dark:
theme = "dark"
Copy link

Choose a reason for hiding this comment

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

logic: dark=False would still trigger this condition, potentially unexpected behavior

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ischemist/plotly.py
Line: 131:132

Comment:
**logic:** dark=False would still trigger this condition, potentially unexpected behavior

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope/domain the core logic, abstractions, and internal workings of the package or application type/refactor internal code quality improvement. no user-facing changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant