NNPDF / pineappl

PineAPPL is not an extension of APPLgrid
https://nnpdf.github.io/pineappl/
GNU General Public License v3.0
12 stars 3 forks source link

Support native pathlike objects #105

Closed alecandido closed 2 years ago

alecandido commented 2 years ago

This should close #84.

It's a tiny PR, and we can even quickly merge. I ran existing tests, and slightly extended (sorry for the diff, but someone forgot to run black on unit tests). I even tested it manually, and it's smoothly working.

Speaking of tests: maybe it might be a good chance to extend the IO related ones a bit (write_lz4 is not tested, nor read, write and write_lz4 for FkTables... actually almost nothing is tested of Fk). In any case: the interface is a thin wrapper, so an extensive test suite is not that required, and we can even postpone to a more consistent tests rework.

codecov[bot] commented 2 years ago

Codecov Report

Merging #105 (5c22c2f) into master (c0144e4) will increase coverage by 0.06%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
+ Coverage   88.40%   88.46%   +0.06%     
==========================================
  Files          31       31              
  Lines        3070     3070              
==========================================
+ Hits         2714     2716       +2     
+ Misses        356      354       -2     
Impacted Files Coverage Δ
pineappl_cli/src/plot.rs 93.15% <0.00%> (+1.05%) :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 c0144e4...5c22c2f. Read the comment docs.

cschwan commented 2 years ago

Looks good to me!

cschwan commented 2 years ago

Speaking of tests: maybe it might be a good chance to extend the IO related ones a bit (write_lz4 is not tested, nor read, write and write_lz4 for FkTables... actually almost nothing is tested of Fk). In any case: the interface is a thin wrapper, so an extensive test suite is not that required, and we can even postpone to a more consistent tests rework.

Agreed, I've opened Issue https://github.com/N3PDF/pineappl/issues/108 for that. Thanks for your work on this Issue @AleCandido!