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

made test_incorrect_doi_period_at_end test match description #81

Closed kyleniemeyer closed 6 years ago

kyleniemeyer commented 6 years ago

[X] Tests fixed: test_incorrect_doi_period_at_end
[x] Added entry into CHANGELOG.md

Changes proposed in this pull request:

Makes test test_incorrect_doi_period_at_end match description

@pr-omethe-us/chemked

codecov[bot] commented 6 years ago

Codecov Report

Merging #81 into master will not change coverage. The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #81   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines         851    851           
  Branches      202    202           
=====================================
  Hits          851    851

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 990d71f...975356a. Read the comment docs.

bryanwweber commented 6 years ago

Do we need this test? What is it testing for?

kyleniemeyer commented 6 years ago

If I recall, some of the ReSpecTh files I saw had periods at the end, and some didn't—so this was just a little test to make sure we can handle that.

bryanwweber commented 6 years ago

But a period at the end of a DOI is a different DOI, no? So if they have a period at the end, its automatically an invalid DOI regardless of why it has a period at the end, unless that period is meant to be stripped off in the processing?

kyleniemeyer commented 6 years ago

Ohh I just figured out/remembered what this test was for... it wasn't about a period at the end of the DOI, but at the end of the reference.

In the case of an invalid reference, the converter puts whatever is given in the detail field in our reference (with a message so the user can manually correct). The period issue is minor, but a sentence is added after the reference data is copied, so the code checks for a period and adds if there isn't one. This was testing the other branch of that if statement, I think.

bryanwweber commented 6 years ago

OK that makes more sense. I rebased so its just one commit to avoid the noise and churn. Merged!