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

Fixing the float resolver #591

Closed
wants to merge 1 commit into from
Closed

Fixing the float resolver #591

wants to merge 1 commit into from

Conversation

arvkonstantin
Copy link

Adding optional plus and minus operators after scientific notation in the float resolver.

In some programming languages, scientific notation is represented without the "+" operator.
For example , in the Scala programming language , floating-point numbers and scientific notation look like this:

println(14266020.870271318)
1.4266020870271318E7

And if the yaml file was generated via Scala, and then parsed by the PyYAML library, then this number is perceived as a string.

I made a minor correction that adds the optional operators after scientific notation.

Adding optional plus and minus operators after scientific notation in the float resolver
@perlpunk
Copy link
Member

perlpunk commented Dec 2, 2021

As you can see here http://yaml.org/type/float.html , for the YAML 1.1 float type the sign is not optional, and PyYAML implements YAML 1.1.
There is a PR for supporting YAML 1.2, where the sign is optional.
Here you can see all type definitions in both versions: https://perlpunk.github.io/yaml-test-schema/schemas.html

@perlpunk
Copy link
Member

perlpunk commented Dec 2, 2021

The PR tests are failing locally for me:

.................................................................................................................................................................................................................................
.................................................................................................................................................................................................................................
.................................................................................................................................................................................................................................
..................................Compare: >>'+0.3e3'<< >>+0.3e3<<
Input: >>!!str +0.3e3<<                       
['str', '+0.3e3', '+0.3e3']               
Compare: >>'.3e3'<< >>.3e3<<               
Input: >>!!str .3e3<<                           
['str', '.3e3', '.3e3']                               
Compare: >>'0.3e3'<< >>0.3e3<<
Input: >>!!str 0.3e3<<                   
['str', '0.3e3', '0.3e3']                
Input: >>+0.3e3<<                             
['str', '+0.3e3', '+0.3e3']                   
Compare: >>300.0<< >>+0.3e3<<                                     
Input: >>+0.3e3<<                                     
['str', '+0.3e3', '+0.3e3']                     
Input: >>.3e3<<                            
['str', '.3e3', '.3e3']                               
Compare: >>300.0<< >>.3e3<<                  
Input: >>.3e3<<                                          
['str', '.3e3', '.3e3']                     
Input: >>0.3e3<<                          
['str', '0.3e3', '0.3e3']                 
Compare: >>300.0<< >>0.3e3<<              
Input: >>0.3e3<<                        
['str', '0.3e3', '0.3e3']                             
Failed 9 / 261 tests                         
F................................................................................................................................................................................................................................
.................................................................................................................................................................................................................................
............................................................................................................................
===========================================================================
test_implicit_resolver(tests/data/yaml11.schema, tests/data/yaml11.schema-skip): FAILURE
Traceback (most recent call last):             
  File "tests/lib/test_appliance.py", line 59, in execute
    function(verbose=verbose, *filenames)
  File "tests/lib/test_schema.py", line 141, in test_implicit_resolver
    assert(False)
AssertionError: see below
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
---------------------------------------------------------------------------
tests/data/yaml11.schema:
# https://github.com/perlpunk/yaml-test-schema/blob/master/data/schema-yaml11.yaml
---
'!!bool FALSE': ['bool', 'false()', 'false']
'!!bool False': ['bool', 'false()', 'false']
...
---------------------------------------------------------------------------
tests/data/yaml11.schema-skip:    
load: {                            
  'Y': 1, 'y': 1, 'N': 1, 'n': 1,
  '!!bool Y': 1, '!!bool N': 1, '!!bool n': 1, '!!bool y': 1,
  }                             
dump: {                           
  '!!str N': 1, '!!str Y': 1, '!!str n': 1, '!!str y': 1,
  }                            
===========================================================================
TESTS: 1283                      
FAILURES: 1                 
error: Tests failed                  

Not sure why github actions succeed.

@nitzmahone any idea? I'm having trouble again finding the part where the actual tests are run :)
https://github.com/yaml/pyyaml/actions/runs/1529387177

@perlpunk
Copy link
Member

perlpunk commented Dec 2, 2021

There are several places where I can find failing tests, but they do not result in a failing workflow step: https://github.com/yaml/pyyaml/runs/4393221891?check_suite_focus=true#step:6:726

Copy link
Member

@perlpunk perlpunk left a comment

Choose a reason for hiding this comment

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

I vote for closing, see my comments.
But it should be investigated why the test failures didn't show up as failures.

@nitzmahone
Copy link
Member

sigh just looked- because the ancient homegrown test runner isn't returning a nonzero exit code on test failures... Yet another reason to standardize: #588

This pull request was closed.
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