Skip to content

fix: improve numeric literal validation#21

Merged
alesanfra merged 3 commits into
toon-format:mainfrom
smortezah:num-detect
Mar 17, 2026
Merged

fix: improve numeric literal validation#21
alesanfra merged 3 commits into
toon-format:mainfrom
smortezah:num-detect

Conversation

@smortezah

Copy link
Copy Markdown
Contributor

This pull request improves numeric literal detection and ensures consistency between encoder and decoder utilities. It refines the is_numeric_literal function to properly handle negative numbers with leading zeros, and introduces a comprehensive test suite to verify numeric detection logic and round-trip encoding/decoding behavior.

Numeric literal detection improvements

  • Updated is_numeric_literal in src/toon_format/_literal_utils.py to correctly reject negative numbers with leading zeros (e.g., -01), ensuring stricter validation of numeric literals.

Test coverage enhancements

  • Added tests/test_numeric_detection.py with extensive tests for both is_numeric_literal and is_numeric_like, covering valid numbers, floats, scientific notation, leading zeros, non-numeric strings, and special float values.
  • Included consistency tests to verify that encoder and decoder utilities agree on valid and invalid numeric representations, especially for octal-like numbers with leading zeros.
  • Added round-trip tests to confirm that encoding and decoding preserve the correct types for valid numbers and octal-like strings, preventing unintended type conversions.

@smortezah
smortezah requested a review from a team as a code owner November 6, 2025 13:09
bpradana
bpradana previously approved these changes Nov 8, 2025

@bpradana bpradana left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

approved!
@Justar96 is it okay if I implement git submodules for the fixtures? So the fixtures are always up to date.
What do you think? 😄

@Justar96

Justar96 commented Nov 8, 2025

Copy link
Copy Markdown
Contributor

@bpradana pls do. Thanks a lot

@davidpirogov
davidpirogov self-requested a review November 9, 2025 03:34
davidpirogov
davidpirogov previously approved these changes Nov 9, 2025
@davidpirogov
davidpirogov dismissed stale reviews from bpradana and themself via 9301d02 November 9, 2025 03:43

@davidpirogov davidpirogov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor update to test asserts, otherwise looks good to me

@davidpirogov
davidpirogov requested a review from bpradana November 9, 2025 03:45
@johannschopplich

Copy link
Copy Markdown
Contributor

@alesanfra this one's been sitting for a while and looks solid to me – happy if you want to take a quick look and merge if it checks out! 🙌

@alesanfra
alesanfra requested a review from a team March 17, 2026 08:45

@alesanfra alesanfra left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@alesanfra
alesanfra merged commit 728120d into toon-format:main Mar 17, 2026
6 checks passed
@smortezah
smortezah deleted the num-detect branch March 17, 2026 09:17
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.

6 participants