Skip to content

Extend parsing of Scientific Notation to JSON capabilites #2365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

maximilian-tech
Copy link

@maximilian-tech maximilian-tech commented Mar 27, 2025

Solves Issues #1955 (comment) and #1952 (comment)

It boils down to yaml vs. json floating point parsing. The SafeLoader of yaml has to be extended to allow for the same range of float notations like json.
See here for more background.
I copied the SO-solution and it does work (for me)

This is needed to postprocess Fortran output as it often as a e (e.g. 1e-5)for scientific notation, which yaml does not accept, but json does accept.
And because the json schema input is parsed through a yaml loader, this notation is not accepted. This PR extends the yaml loader to accept json notation of scientific data representation.

@maximilian-tech maximilian-tech force-pushed the Fix_parsing_scientific_notation branch 3 times, most recently from 64b0300 to 7a0ccf9 Compare March 27, 2025 17:46
@maximilian-tech maximilian-tech force-pushed the Fix_parsing_scientific_notation branch from f0c46a6 to 1852aa9 Compare March 27, 2025 17:49
@maximilian-tech
Copy link
Author

maximilian-tech commented Mar 27, 2025

@gaborbernat I rebased and added the CLI option --extend-yaml-scientifc-notation.

The current implementation uses a global variable, but this is the best solution I could come up with. Ideally the command line option is gone in the '1.0' release and with it this 'construct' so I think it is ok for now.

Copy link
Collaborator

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Can you please add tests for the new features?

@maximilian-tech maximilian-tech force-pushed the Fix_parsing_scientific_notation branch from dfec94d to 0a78f68 Compare March 28, 2025 20:26
@maximilian-tech
Copy link
Author

maximilian-tech commented Mar 28, 2025

I added one test.

The pre-commit.ci bot does quite a lot of code changes I don't know how to prevent. This should probably be changed.

@maximilian-tech maximilian-tech force-pushed the Fix_parsing_scientific_notation branch from f1ec14a to 68dc5a5 Compare March 28, 2025 20:30
@maximilian-tech maximilian-tech force-pushed the Fix_parsing_scientific_notation branch from a06ce12 to c99014c Compare March 28, 2025 20:31
@maximilian-tech maximilian-tech force-pushed the Fix_parsing_scientific_notation branch from 25ab1a4 to 47b2eea Compare March 28, 2025 20:35
@maximilian-tech
Copy link
Author

maximilian-tech commented Apr 14, 2025

Is there anything I can do to aid the progress here? I need this to parse json schema which contains Fortran style scientific notation ( e.g. 1e-5) which is butchered by the yaml loader.

Copy link
Collaborator

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

CI Failing.

@maximilian-tech
Copy link
Author

Yes, because the pre-commit bot breaks the code.

@gaborbernat
Copy link
Collaborator

gaborbernat commented Apr 28, 2025

Yes, because the pre-commit bot breaks the code.

You'll need to address that 🤔 by overriding the ruff checks causing the issue.

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.

3 participants