Skip to content

feat: add assertion for light/dark color list length parity#55

Open
deependujha wants to merge 1 commit into
Lightning-AI:mainfrom
deependujha:fix/assert-color-list-length-parity
Open

feat: add assertion for light/dark color list length parity#55
deependujha wants to merge 1 commit into
Lightning-AI:mainfrom
deependujha:fix/assert-color-list-length-parity

Conversation

@deependujha
Copy link
Copy Markdown

What does this PR do?

Adds a module-level assertion to verify that _LIGHT_THEME_COLORS and
_DARK_THEME_COLORS have the same length.

Why is this needed?

_create_colors computes an index against len(_DARK_THEME_COLORS) and
uses it to index into both lists. If the two lists ever diverge in length
due to an accidental edit, the function would silently return a mismatched
color pair or raise an IndexError at runtime with no clear indication of
the root cause.

A module-level assertion catches this at import time, failing loudly and
immediately.

Changes

  • Added assert len(_LIGHT_THEME_COLORS) == len(_DARK_THEME_COLORS) with
    a descriptive error message after the color list definitions in
    colors.py

Copy link
Copy Markdown
Member

@justusschock justusschock left a comment

Choose a reason for hiding this comment

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

Assertions in production code should be avoided unless strictly required.

@deependujha
Copy link
Copy Markdown
Author

So, are you in favour of using if-cond instead of assert, or close it altogether?

Only issue being, it could mistakenly be updated to have different lengths and silent bugs.

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