PEtab-dev / libpetab-python

Python package for working with PEtab files
https://libpetab-python.readthedocs.io
MIT License
14 stars 6 forks source link

Add support for pathlib for reading PEtab tables #93

Closed dweindl closed 2 years ago

dweindl commented 2 years ago

Closes #82

codecov-commenter commented 2 years ago

Codecov Report

Merging #93 (f1c709d) into develop (4f6b841) will increase coverage by 0.04%. The diff coverage is 90.56%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #93      +/-   ##
===========================================
+ Coverage    79.65%   79.70%   +0.04%     
===========================================
  Files           26       26              
  Lines         2935     2941       +6     
  Branches       709      709              
===========================================
+ Hits          2338     2344       +6     
  Misses         422      422              
  Partials       175      175              
Impacted Files Coverage Δ
petab/problem.py 64.47% <33.33%> (ø)
petab/yaml.py 90.78% <92.30%> (ø)
petab/conditions.py 90.69% <100.00%> (+0.22%) :arrow_up:
petab/core.py 87.02% <100.00%> (+0.09%) :arrow_up:
petab/measurements.py 82.02% <100.00%> (+0.20%) :arrow_up:
petab/observables.py 97.14% <100.00%> (+0.04%) :arrow_up:
petab/parameters.py 86.53% <100.00%> (+0.08%) :arrow_up:
petab/sbml.py 57.97% <100.00%> (+0.30%) :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 4f6b841...f1c709d. Read the comment docs.

dweindl commented 2 years ago

Maybe nice to define e.g. petab.C.TYPE_PATH = Union[str, Path], could make the type hints a little neater. Not sure if it can be used as isinstance(..., TYPE_PATH) though.

Does not look very neat, agreed. Briefly thought about it, but then decided that it might be more convenient to users to directly have the types there, without looking up what the other type is...

dilpath commented 2 years ago

Ah, additionally, the tests were changed to use Path, so maybe str is no longer tested? Fine as is though