Add enum type conversion tests and update data_types.py for enum hand…#59
Add enum type conversion tests and update data_types.py for enum hand…#59
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves confkit’s automatic default-type casting so that enum members used as Config defaults are wrapped in the appropriate enum BaseDataType converters (instead of being treated as plain str/int), and adds tests to prevent regressions.
Changes:
- Update
BaseDataType.cast()to detectStrEnum/IntEnum/IntFlag/Enumdefaults before primitivestr/intmatching. - Add a new test module asserting that enum defaults are auto-wrapped into the correct confkit enum data type converters.
- Add a Ruff per-file ignore (E701) to allow single-line
match/casereturns indata_types.py.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
tests/test_enum_auto_conversion.py |
Adds regression tests validating that enum-member defaults are auto-wrapped into confkit’s enum data types. |
src/confkit/data_types.py |
Adjusts BaseDataType.cast() pattern matching order to correctly handle enum instances before str/int. |
ruff.toml |
Adds a per-file lint ignore to permit the new single-line case ...: return ... style. |
| """Test that StrEnum instances are automatically wrapped.""" | ||
|
|
||
| def test_strenum_auto_wraps_to_config_strenum(self, tmp_path: Path) -> None: | ||
| """Test that Config(StrEnum.value) auto-wraps to Config(ConfigStrEnum(StrEnum.value)).""" |
There was a problem hiding this comment.
This docstring mentions wrapping Config(StrEnum.value), but the test passes an enum member (LogLevel.INFO) rather than .value. Consider updating the wording to reflect the actual input being asserted (auto-wrapping for enum members).
| """Test that IntEnum instances are automatically wrapped.""" | ||
|
|
||
| def test_intenum_auto_wraps_to_config_intenum(self, tmp_path: Path) -> None: | ||
| """Test that Config(IntEnum.value) auto-wraps to Config(ConfigIntEnum(IntEnum.value)).""" |
There was a problem hiding this comment.
This docstring mentions wrapping Config(IntEnum.value), but the test passes an enum member (Priority.MEDIUM) rather than .value. Consider updating the wording to reflect the actual input being asserted (auto-wrapping for enum members).
| """Test that IntFlag instances are automatically wrapped.""" | ||
|
|
||
| def test_intflag_auto_wraps_to_config_intflag(self, tmp_path: Path) -> None: | ||
| """Test that Config(IntFlag.value) auto-wraps to Config(ConfigIntFlag(IntFlag.value)).""" |
There was a problem hiding this comment.
This docstring mentions wrapping Config(IntFlag.value), but the test passes an enum member (Permission.READ) rather than .value. Consider updating the wording to reflect the actual input being asserted (auto-wrapping for enum members).
| """Test that Config(IntFlag.value) auto-wraps to Config(ConfigIntFlag(IntFlag.value)).""" | |
| """Test that Config(IntFlag member) auto-wraps to Config(ConfigIntFlag(IntFlag member)).""" |
| """Test that standard Enum instances are automatically wrapped.""" | ||
|
|
||
| def test_standard_enum_auto_wraps_to_config_enum(self, tmp_path: Path) -> None: | ||
| """Test that Config(Enum.value) auto-wraps to Config(ConfigEnum(Enum.value)).""" |
There was a problem hiding this comment.
This docstring mentions wrapping Config(Enum.value), but the test passes an enum member (StandardEnum.OPTION_A) rather than .value. Consider updating the wording to reflect the actual input being asserted (auto-wrapping for enum members).
| [lint.per-file-ignores] | ||
| "test_*.py" = ["S101", "D103", "SLF001", "D100","D101", "FBT003", "PLR2004", "N814", "S105"] | ||
| "examples/*.py" = ["ALL"] | ||
| "src/confkit/data_types.py" = ["E701"] # Allow inline return after : on match for readability |
There was a problem hiding this comment.
The new per-file ignore for E701 is only needed because match cases in BaseDataType.cast() are written as single-line case ...: return .... Instead of weakening linting for a core source file, consider formatting the case bodies onto the next line and removing this ignore so the codebase keeps consistent lint enforcement for src/ modules.
| "src/confkit/data_types.py" = ["E701"] # Allow inline return after : on match for readability |
| case dStrEnum(): return cast("BaseDataType[T]", StrEnum(default)) | ||
| case dIntFlag(): return cast("BaseDataType[T]", IntFlag(default)) | ||
| case dIntEnum(): return cast("BaseDataType[T]", IntEnum(default)) | ||
| case dEnum(): return cast("BaseDataType[T]", Enum(default)) | ||
| case bool(): return cast("BaseDataType[T]", Boolean(default)) | ||
| case None: return cast("BaseDataType[T]", NoneType()) | ||
| case int(): return cast("BaseDataType[T]", Integer(default)) | ||
| case float(): return cast("BaseDataType[T]", Float(default)) | ||
| case str(): return cast("BaseDataType[T]", String(default)) | ||
| case BaseDataType(): return default |
There was a problem hiding this comment.
BaseDataType.cast() uses single-line case ...: return ... statements, which required adding a Ruff E701 ignore for this production file. Consider expanding each case to a multi-line block (return on the next line) and dropping the ignore; that keeps lint rules consistent and avoids allowing other future one-line multi-statement patterns in this module.
| case dStrEnum(): return cast("BaseDataType[T]", StrEnum(default)) | |
| case dIntFlag(): return cast("BaseDataType[T]", IntFlag(default)) | |
| case dIntEnum(): return cast("BaseDataType[T]", IntEnum(default)) | |
| case dEnum(): return cast("BaseDataType[T]", Enum(default)) | |
| case bool(): return cast("BaseDataType[T]", Boolean(default)) | |
| case None: return cast("BaseDataType[T]", NoneType()) | |
| case int(): return cast("BaseDataType[T]", Integer(default)) | |
| case float(): return cast("BaseDataType[T]", Float(default)) | |
| case str(): return cast("BaseDataType[T]", String(default)) | |
| case BaseDataType(): return default | |
| case dStrEnum(): | |
| return cast("BaseDataType[T]", StrEnum(default)) | |
| case dIntFlag(): | |
| return cast("BaseDataType[T]", IntFlag(default)) | |
| case dIntEnum(): | |
| return cast("BaseDataType[T]", IntEnum(default)) | |
| case dEnum(): | |
| return cast("BaseDataType[T]", Enum(default)) | |
| case bool(): | |
| return cast("BaseDataType[T]", Boolean(default)) | |
| case None: | |
| return cast("BaseDataType[T]", NoneType()) | |
| case int(): | |
| return cast("BaseDataType[T]", Integer(default)) | |
| case float(): | |
| return cast("BaseDataType[T]", Float(default)) | |
| case str(): | |
| return cast("BaseDataType[T]", String(default)) | |
| case BaseDataType(): | |
| return default |
| def test_strenum_auto_wraps_to_config_strenum(self, tmp_path: Path) -> None: | ||
| """Test that Config(StrEnum.value) auto-wraps to Config(ConfigStrEnum(StrEnum.value)).""" | ||
| config_file = tmp_path / "config.ini" | ||
| config_file.write_text("") | ||
|
|
||
| class StrEnumConfig(ConfigBase): | ||
| pass | ||
|
|
||
| StrEnumConfig.set_parser(IniParser()) | ||
| StrEnumConfig.set_file(config_file) | ||
| StrEnumConfig._has_read_config = False |
There was a problem hiding this comment.
The setup boilerplate (create config file, define a Config subclass, set parser/file, reset _has_read_config) is repeated across the four tests. Consider using a shared fixture/helper or a single parametrized test to reduce duplication and make it easier to add more enum variants later.
…ling