Refactor MetaData class: reduce duplication and improve code quality#52
Merged
Merged
Conversation
- Extract __filter_by_version() helper for version/deprecation filtering logic that was repeated in 5 of 6 parsing methods - Extract __extract_common() helper for dimensions/modules/requires extraction repeated in 4 methods - Extract __extract_valid_values() helper for valid values parsing repeated in 2 methods - Remove ~40 lines of commented-out dead code - No behavioral changes; all metadata output is structurally identical
- Add 'from __future__ import annotations' for runtime compatibility - Replace typing.Dict with built-in dict - Replace Union[str, Version] with str | Version - Remove unused typing imports (Dict, Optional, Union)
- Remove unnecessary '# type: ignore' on xml.etree.ElementTree import - Add __repr__ method for debugging - Move outside_elem module-level dict to _OUTSIDE_ELEM class constant for better encapsulation (was only used internally)
- Add __find_text() helper that safely returns element text or None - Replace all 'elem.find(ev).text' patterns wrapped in try/except AttributeError with explicit None checks using __find_text() - Makes control flow clearer: missing elements are handled with 'if text is None' rather than relying on exception handling - No behavioral changes
- Replace bool() with a lambda that checks for '1', 'True', 'true'
- bool('0') and bool('False') both return True in Python, which would
be incorrect if the XML ever uses those values
- Current XML only uses '1' for fixed dimensions and omits the element
otherwise, so this is a defensive fix
- Move NEW_DTYPE and NEW_PARAM_DTYPE from metadata.py to constants.py alongside the other type-mapping dicts (NEW_PTYPE_TO_DTYPE, etc.) - Add proper Sphinx-style docstring to __init__ - Add docstring to metadata property
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor MetaData class: reduce duplication and improve code quality
Summary
Major refactoring of
metadata.pyto reduce code duplication, modernize type hints, fix a latent bug, and improve readability.Changes
Reduce duplication (largest change)
__filter_by_version()helper for version/deprecation filtering logic that was repeated in 5 of 6 parsing methods__extract_common()helper for dimensions/modules/requires extraction repeated in 4 methods__extract_valid_values()helper for valid values parsing repeated in 2 methodsModernize type hints
from __future__ import annotationstyping.Dictwith built-indictUnion[str, Version]withstr | VersiontypingimportsReplace implicit exception handling with explicit None checks
__find_text()helper that safely returns element text or Noneelem.find(ev).textpatterns wrapped intry/except AttributeErrorwith explicitif text is NonechecksFix
is_fixedbool parsingbool()with a lambda that checks for'1','True','true'bool('0')andbool('False')both returnTruein Python, which would be incorrect if the XML ever uses those values'1'for fixed dimensions)Minor cleanup
# type: ignoreonxml.etree.ElementTreeimport__repr__method for debuggingoutside_elemmodule-level dict to_OUTSIDE_ELEMclass constantNEW_DTYPEandNEW_PARAM_DTYPEdicts toconstants.pyalongside other type-mapping dicts__init__andmetadatapropertyTesting
All 290 tests pass. Metadata output verified structurally identical across versions 4, 5.2.1.1, 6.0.0, and 60.0.