search

erwanp / qtplaskin

A graphical tool to explore ZdPlaskin Results
GNU Lesser General Public License v3.0
7 stars 5 forks source link

Correction two-letters atom problem, 3rd try #16

Closed maillardj closed 4 years ago

maillardj commented 4 years ago
  1. Modification in database.py to prevent failure when molar mass is unknown + test
  2. Modification in modeldata.py to be able to recognize two-letters atoms + test on Ar
codecov[bot] commented 4 years ago

Codecov Report

Merging #16 into master will increase coverage by 1.47%. The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
+ Coverage   22.40%   23.87%   +1.47%     
==========================================
  Files          18       19       +1     
  Lines        1754     1780      +26     
==========================================
+ Hits          393      425      +32     
+ Misses       1361     1355       -6
Impacted Files Coverage Δ
qtplaskin/database.py 83.87% <88.88%> (+4.70%) :arrow_up:
test/test_two_letters_atom_failure.py 92.85% <92.85%> (ø)
qtplaskin/modeldata.py 39.71% <100.00%> (+1.18%) :arrow_up:
test/test_02.py 100.00% <0.00%> (+18.75%) :arrow_up:

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 85fd959...9a8302c. Read the comment docs.

erwanp commented 4 years ago

Good, all tests pass !

Only comment : there is warning in the Tests : https://travis-ci.com/github/erwanp/qtplaskin/builds/168183048 that says 

test/test_two_letters_atom_failure.py::test_if_Ar_is_recognized
  /home/travis/build/erwanp/qtplaskin/qtplaskin/database.py:58: UserWarning: Unknown molar mass for specie 'E'
    warn('Unknown molar mass for specie '+str(err))

Given it's electrons and there will be electrons in almost every simulation, shouldnt we ignore this ?