Skip to content

feat: generalize config float parsing#488

Open
agitter wants to merge 1 commit into
mainfrom
yaml-float
Open

feat: generalize config float parsing#488
agitter wants to merge 1 commit into
mainfrom
yaml-float

Conversation

@agitter

@agitter agitter commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Closes #484

This implements two fixes for the limited syntax supported by the default YAML float specification: https://yaml.org/type/float.html add_implicit_resolver is used when we load a YAML file directly. sanitize_scientific_notation is used when Snakemake loads the config file because we cannot modify it's YAML loader soon enough.

I used Gemini to help plan the patch and draft code.

@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 spras | 🛠️ Build #33120127 | 📁 Comparing b1b54a1 against latest (d990664)

  🔍 Preview build  

3 files changed
± genindex.html
± fordevs/spras.config.html
± fordevs/spras.html

@agitter agitter mentioned this pull request Jun 13, 2026
Comment thread spras/config/config.py
from spras.util import LoosePathLike, NpHashEncoder, hash_params_sha1_base32

# Modify YAML float specification when a YAML file is parsed directly
# Default requires decimal point for scientific notation: https://yaml.org/type/float.html

@tristan-f-r tristan-f-r Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While the YAML specification itself doesn't support this, Pydantic handles coercion of floats from strings just fine, including with scientific notation. This error was instead caused by my assumption that this coercion would happen before any BeforeValidator was run, which is notably the opposite of the characterization of BeforeValidator: I've proposed an alternative fix instead.

@ntalluri ntalluri Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The alternative fix is very clean, I am curious if it will pass the tests Tony set up.

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.

Config file parsing breaks for scientific notation

3 participants