refactor: random non-important change - #12
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors LogCanonicalizer to return the canonicalized lines, renames _LogSection.Other to _LogSection.OTHER, and moves _LogSection and _match_section to the bottom of the file. It also updates imports in main.py and updates the CLI description in parser.py. However, moving _LogSection and _match_section to the bottom of the file introduces a runtime NameError because they are referenced in type annotations before they are defined. This should be resolved by moving them back to the top or using from __future__ import annotations.
| class _LogSection(StrEnum): | ||
| DOC = "Doc" | ||
| PAPER = "Paper" | ||
| WIKI = "Wiki" | ||
| OTHER = "Other" | ||
|
|
||
|
|
||
| def _match_section( | ||
| line: str, section_patterns: dict[_LogSection, re.Pattern] | ||
| ) -> _LogSection | None: | ||
| """Return the log section type if the line matches any section pattern, else None.""" | ||
| for section_type, pattern in section_patterns.items(): | ||
| if pattern.fullmatch(line): | ||
| return section_type | ||
| return None |
There was a problem hiding this comment.
Moving _LogSection and _match_section to the bottom of the file causes a NameError at runtime when importing this module.\n\nIn Python, type annotations in function/method signatures (such as log_section: _LogSection in _canonicalize_line and _canonicalize_item) are evaluated when the class/function is defined. Since _LogSection is now defined at the bottom of the file (after LogCanonicalizer), importing canonicalizer.py will fail with:\nNameError: name '_LogSection' is not defined\n\nTo resolve this, please move _LogSection and _match_section back to the top of the file (before LogCanonicalizer), or import from __future__ import annotations at the very top of the file.
There was a problem hiding this comment.
Pull request overview
This PR updates the CLI argument parser messaging and refactors log canonicalization to return the processed lines, updating the main entrypoint accordingly.
Changes:
- Updated CLI description text in the argument parser.
- Switched
main.pyto importparse_argsfromvanillian.parserand to use the return value ofLogCanonicalizer.canonicalize(). - Refactored
LogCanonicalizer.canonicalize()to returnlist[str]and normalized_LogSection.OTHERnaming.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vanillian/parser.py | Updates CLI description string. |
| src/vanillian/main.py | Fixes parse_args import and uses returned canonicalized lines. |
| src/vanillian/log/canonicalizer.py | Changes canonicalize() to return lines and renames _LogSection member; moves enum/helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class LogCanonicalizer: | ||
| def __init__(self) -> None: | ||
| self.section_patterns = { | ||
| _LogSection.DOC: re.compile(r"\\\[Doc\\\]\s"), | ||
| _LogSection.PAPER: re.compile(r"\\\[Paper\\\]\s"), | ||
| _LogSection.WIKI: re.compile(r"\\\[Wiki\\\]\s"), | ||
| _LogSection.Other: re.compile(r"\\\[.+\\\]\s"), | ||
| _LogSection.OTHER: re.compile(r"\\\[.+\\\]\s"), |
| def canonicalize(self, lines: list[str], *, force: bool = False) -> list[str]: | ||
| """Canonicalize log items in the provided lines. | ||
|
|
||
| Modifies the input lines in place. | ||
|
|
||
| Args: | ||
| lines (list[str]): The lines of the log file. | ||
| force (bool, optional): If True, canonicalize without asking for confirmation. Defaults to False. | ||
|
|
||
| Returns: | ||
| list[str]: The canonicalized log lines. | ||
| """ | ||
| logger.info("Starting log canonicalization.") | ||
| current_section = _LogSection.Other | ||
|
|
||
| current_section = _LogSection.OTHER | ||
| for idx, line in enumerate(lines): | ||
| if (match := _match_section(line, self.section_patterns)) is not None: | ||
| level = logger.info if current_section != _LogSection.Other else logger.debug | ||
| level = logger.info if current_section != _LogSection.OTHER else logger.debug | ||
| level("Line: %d: Entering section: '%s'", idx, match.value) | ||
| current_section = match | ||
| elif self.log_item_pattern.match(line) and current_section != _LogSection.Other: | ||
| elif self.log_item_pattern.match(line) and current_section != _LogSection.OTHER: | ||
| logger.info("Line: %d: Processing...", idx) | ||
| lines[idx] = self._canonicalize_line(current_section, line, force=force) | ||
| logger.info("Line: %d: Finished processing.", idx) |
| class _LogSection(StrEnum): | ||
| DOC = "Doc" | ||
| PAPER = "Paper" | ||
| WIKI = "Wiki" | ||
| OTHER = "Other" | ||
|
|
||
|
|
||
| def _match_section( | ||
| line: str, section_patterns: dict[_LogSection, re.Pattern] | ||
| ) -> _LogSection | None: |
| Modify the input lines in place. | ||
|
|
||
| Args: | ||
| lines (list[str]): The lines of the log file. | ||
| force (bool, optional): If True, canonicalize without asking for confirmation. Defaults to False. | ||
|
|
||
| Returns: | ||
| list[str]: The canonicalized log lines. |
No description provided.