Skip to content
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

Fix resolving scientific notation #174

Closed
wants to merge 1 commit into from
Closed

Fix resolving scientific notation #174

wants to merge 1 commit into from

Conversation

coldfix
Copy link

@coldfix coldfix commented Jun 19, 2018

Resolves #173.

fixes that scientific notation without dot in the mantissa is not resolved as float, as would be requested by YAML 1.2 spec.

Best, Thomas

@coldfix
Copy link
Author

coldfix commented Jun 24, 2018

Hi, FYI I have fixed the failing test. The py33 test seem to be erroring due to a technical issue in the test environment.

@jhagege
Copy link

jhagege commented Jul 18, 2018

Great fix, would be happy to see it merged to next release !
I am struggling with the same issue.

@AvdN
Copy link

AvdN commented Jul 18, 2018

I was pointed out this PR through a comment on the stackexchange issue, and my 2 cents are that there are multiple problems with this PR.

First of all PyYAML is documented to be a YAML 1.1 parser, so it should do so for explicit %YAML 1.1 documents as well as for implicit documents. If you do YAML 1.2 support make sure YAML 1.1 is still supported. Partial YAML 1.2 behavior on can of course be supported by adding some optional parameter to the load family of functions, but should never be the default.

I would also expect some extra tests to be part of the commit. Tests that show failure of the format on default (1.1) mode and appropriate processing on partial 1.2 mode.

Allow exponents without sign, mantissa without dot.

Resolves #173
@coldfix
Copy link
Author

coldfix commented Jul 19, 2018

First of all PyYAML is documented to be a YAML 1.1 parser

good point. I didn't even notice and somehow assumed it would do YAML 1.2 since that's been out for long enough.

If you do YAML 1.2 support make sure YAML 1.1 is still supported. Partial YAML 1.2 behavior on can of course be supported by adding some optional parameter to the load family of functions, but should never be the default.

Another, perhaps simpler, possibility could be to just make PyYAML 4.0 officially a YAML 1.2 parser and merge this PR into there?

BTW: As pointed out by @perlpunk in #173, the dot is optional in YAML 1.2 as well, so I squashed both commits into one.

cmcantalupo added a commit to cmcantalupo/geopm that referenced this pull request Mar 25, 2020
- Due to error in pyyaml scientific notation floating
  point numbers without a decimal point are parsed as
  strings:  e.g. "1e+09".
- Fix is based on PR:
  <yaml/pyyaml#174>
  to the pyyaml project.

Change-Id: I5cc4afc0d6eecf204b92ba111185d8cc30dc6c5d
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
cmcantalupo added a commit to cmcantalupo/geopm that referenced this pull request Mar 25, 2020
- Due to error in pyyaml scientific notation floating
  point numbers without a decimal point are parsed as
  strings:  e.g. "1e+09".
- Fix is based on PR:
  <yaml/pyyaml#174>
  to the pyyaml project.

Change-Id: I5cc4afc0d6eecf204b92ba111185d8cc30dc6c5d
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
cmcantalupo added a commit to cmcantalupo/geopm that referenced this pull request Mar 25, 2020
- Due to error in pyyaml scientific notation floating
  point numbers without a decimal point are parsed as
  strings:  e.g. "1e+09".
- Fix is based on PR:
  <yaml/pyyaml#174>
  to the pyyaml project.

Change-Id: I5cc4afc0d6eecf204b92ba111185d8cc30dc6c5d
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
dannosliwcd pushed a commit to geopm/geopm that referenced this pull request Mar 25, 2020
- Due to error in pyyaml scientific notation floating
  point numbers without a decimal point are parsed as
  strings:  e.g. "1e+09".
- Fix is based on PR:
  <yaml/pyyaml#174>
  to the pyyaml project.

Change-Id: I5cc4afc0d6eecf204b92ba111185d8cc30dc6c5d
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
@bronger
Copy link

bronger commented Oct 30, 2020

Any progress on this? PyYAML generates invalid hashes after all! Currenly we are bitten by a Git commit that happens to be called 45433e8, and is imported into Kubernetes as 4543300000000. FWIW, I don’t think @AvdN objection justifies this still being delayed.

@fujitatomoya
Copy link

@coldfix r u still working on this? there seems to be conflicts to the mainline.

@coldfix
Copy link
Author

coldfix commented Jun 23, 2023

Hey @fujitatomoya,

not actively as there doesn't seem to be any interest in upstream for merging this. If an update is requested I might look into it. No use in regularly resolving conflicts otherwise.

@coldfix
Copy link
Author

coldfix commented Jun 30, 2023

Superseeded by #555.

@coldfix coldfix closed this Jun 30, 2023
@coldfix coldfix deleted the fix-scientific-notation branch July 1, 2023 11:55
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.

Numbers in scientific notation without dot are parsed as string
6 participants