lmaurits / BEASTling

A linguistics-focussed command line tool for generating BEAST XML files.
BSD 2-Clause "Simplified" License
20 stars 6 forks source link

Minor fixes #211

Closed Anaphory closed 5 years ago

Anaphory commented 5 years ago

This pull request suggests three minor fixes:

If I find the opportunity to add tests or other documentation fixes (eg. #202 #209)

This is a duplicate of #210, except now I picked the correct base branch here.

codecov-io commented 5 years ago

Codecov Report

Merging #211 into develop will increase coverage by 0.76%. The diff coverage is 94.11%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #211      +/-   ##
===========================================
+ Coverage     91.9%   92.66%   +0.76%     
===========================================
  Files           21       21              
  Lines         2605     2577      -28     
===========================================
- Hits          2394     2388       -6     
+ Misses         211      189      -22
Impacted Files Coverage Δ
beastling/__main__.py 0% <0%> (ø) :arrow_up:
beastling/models/covarion.py 98.76% <100%> (+12.31%) :arrow_up:
beastling/models/binary.py 90.65% <100%> (+5.8%) :arrow_up:
beastling/cli.py 90.36% <100%> (+0.11%) :arrow_up:
beastling/configuration.py 92% <0%> (+0.14%) :arrow_up:
beastling/beastxml.py 88.52% <0%> (+0.44%) :arrow_up:
beastling/models/basemodel.py 95.1% <0%> (+0.81%) :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 2960b02...792c20c. Read the comment docs.

Anaphory commented 5 years ago

I don't know how I would reasonably test the __main__ error handling procedure. Any suggenstions, @lmaurits and @xrotwang, or may I merge this as-is?

xrotwang commented 5 years ago

AFAICT, the only course of action taken by BEASTling upon encountering the new exception class, is terminating execution. So I guess, no tests are required, there either will be regular script termination or an error will be thrown - and the script will terminate.

Anaphory commented 5 years ago

The idea is that this new Exception subclass is different from existing subclasses, so errors in

try:
   ...
except wrap_errors as e:
    errmsg("Error encountered while parsing configuration file:\n")
    traceback.print_exc()
    sys.exit(2)

are only caught when cli.wrap_errors has its default value Exception, not when __main__ came along and redefined cli.wrap_errors, but the result (termination, which is the desired effect) is exactly as you describe. (NoneException will not be raised.)