Fix detection of dataset column types #10
Conversation
Summary by CodeRabbit
WalkthroughThe changes standardize dataset column identifiers by introducing uppercase string constants and corresponding type aliases in one module, and updating import statements and references in another module to use these new constants. The control flow and logic remain unchanged, focusing solely on improving consistency and type safety. Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
maxim/models/dataset.py (1)
74-82: 🧹 Nitpick (assertive)Literal type aliases mirror column constants effectively
UsingLiteraltypes ensures compile-time safety while preserving the exact runtime values needed for equality checks.Consider adding an
__all__definition to explicitly declare these constants and aliases as the module’s public API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
maxim/dataset/dataset.py(2 hunks)maxim/models/dataset.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
maxim/dataset/dataset.py
25-28: Create your own exception
(TRY002)
25-28: Avoid specifying long messages outside the exception class
(TRY003)
29-29: Unnecessary else after raise statement
Remove unnecessary else
(RET506)
33-36: Create your own exception
(TRY002)
33-36: Avoid specifying long messages outside the exception class
(TRY003)
37-37: Unnecessary else after raise statement
Remove unnecessary else
(RET506)
🪛 Pylint (3.3.7)
maxim/dataset/dataset.py
[refactor] 24-30: Unnecessary "else" after "raise", remove the "else" and de-indent the code inside it
(R1720)
[warning] 25-28: Raising too general exception: Exception
(W0719)
[refactor] 32-38: Unnecessary "else" after "raise", remove the "else" and de-indent the code inside it
(R1720)
[warning] 33-36: Raising too general exception: Exception
(W0719)
🔇 Additional comments (2)
maxim/models/dataset.py (1)
66-72: Introduction of standardized column constants
The new uppercase constants correctly encapsulate dataset column identifiers at runtime and align with the updated comparison logic in the dataset utilities.maxim/dataset/dataset.py (1)
5-5: Imports updated to use new column constants
Switching from PascalCase type aliases to the uppercase string constants (INPUT_COLUMN,EXPECTED_OUTPUT_COLUMN,CONTEXT_TO_EVALUATE_COLUMN) unifies usage across modules.Also applies to: 7-7, 8-8
2 participants
Summary
Testing
pytest -q(fails: ModuleNotFoundError: No module named 'filetype')https://chatgpt.com/codex/tasks/task_e_683ff3fb688c8320975217d49f2acb6f