Skip to content

Comments

Fixing the float resolver#591

Closed
arvkonstantin wants to merge 1 commit intoyaml:masterfrom
arvkonstantin:master
Closed

Fixing the float resolver#591
arvkonstantin wants to merge 1 commit intoyaml:masterfrom
arvkonstantin:master

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