SunSpecOrangeButton / pyoblib

Orange Button Python Library
Apache License 2.0
14 stars 19 forks source link

Implement OB error class in taxonomy_units #138

Closed cwhanse closed 5 years ago

cwhanse commented 5 years ago

Also, some edits to method logic for clarity

cwhanse commented 5 years ago

I'm not sure I follow. If unit_str isn't a valid unit_id, unit_name or id then get_unit will raise the error and terminate. I don't think it will ever pass None back as the unit. Are you saying that you'd prefer that get_unit pass back None and an error message, rather than raising the error?

cwhanse commented 5 years ago

An alternative logic for cli.py:

def list_unit_details(args):
    try:
        unit = taxonomy.units.get_unit(args.unit)
         print...
    except OBNotFoundError:
          print("Not found")
joelebwf commented 5 years ago

The alternative logic looks fine to me. Feel free to implement and merge.

I originally programmed the get_ statements to pas None for "Not Found". That is not necessarily best - just the way I did things. I think we should just keep the discussion going on how to return errors.

cwhanse commented 5 years ago

OK. The other calls to get_unit in cli.list_units_details are OK, since they are within a loop over a list of taxonomy units, rather than some other input.

codecov-io commented 5 years ago

Codecov Report

Merging #138 into master will increase coverage by 0.22%. The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #138      +/-   ##
==========================================
+ Coverage   89.49%   89.72%   +0.22%     
==========================================
  Files          25       25              
  Lines        2466     2462       -4     
==========================================
+ Hits         2207     2209       +2     
+ Misses        259      253       -6
Impacted Files Coverage Δ
oblib/taxonomy_units.py 84.74% <50%> (+4.41%) :arrow_up:
oblib/validator.py 83.33% <0%> (ø) :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 81e3cda...43a738c. Read the comment docs.