pr-omethe-us / PyKED

Python interface to the ChemKED database format
https://pr-omethe-us.github.io/PyKED/
BSD 3-Clause "New" or "Revised" License
15 stars 15 forks source link

Add ignition type #35

Closed kyleniemeyer closed 7 years ago

kyleniemeyer commented 7 years ago

Changes proposed in this pull request:

@pr-omethe-us/chemked

kyleniemeyer commented 7 years ago

OK, that failure is not actually a test failure, but something with ruamel.yaml, and only in the Python 3.5 build...

screen shot 2017-02-27 at 1 08 58 am
codecov-io commented 7 years ago

Codecov Report

Merging #35 into master will increase coverage by 0.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
+ Coverage   96.74%   96.76%   +0.01%     
==========================================
  Files           5        5              
  Lines         246      247       +1     
  Branches       74       74              
==========================================
+ Hits          238      239       +1     
  Misses          4        4              
  Partials        4        4
Impacted Files Coverage Δ
pyked/validation.py 93.1% <100%> (ø) :white_check_mark:
pyked/chemked.py 100% <100%> (ø) :white_check_mark:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 89dedf2...6dd7a4f. Read the comment docs.

bryanwweber commented 7 years ago

It seems like the failure is due to pylint... does it work for you locally?

kyleniemeyer commented 7 years ago

I get the same error locally (this is what I get for running the pytest test suite, but not pylint...).

Apparently this is a pylint/ruamel.yaml conflict: https://bugs.launchpad.net/ubuntu/+source/pylint/+bug/1651098

You can recreate by simply doing:

echo "import ruamel.yaml" > test.py
pylint --reports=no test.py
bryanwweber commented 7 years ago

The failure now is because of 2 things:

  1. the DOI check for the reference in the new test is failing with the wrong year
  2. there's a bug in how the common-properties are implemented. I filed #36 to address this
bryanwweber commented 7 years ago

Awesome, once we merge the uncertainties, we just need to make sure the pressure gets an uncertainty in common-properties too