xgcm / xhistogram

Fast, flexible, label-aware histograms for numpy and xarray
https://xhistogram.readthedocs.io
MIT License
90 stars 20 forks source link

Created checking function for xarray.DataArray and throw an error if not #54

Closed Badboy-16 closed 3 years ago

Badboy-16 commented 3 years ago

I had a go to fix #14

In the histogram() (xhist()) function, I added some code to check if the object passed to the function is an instance of xarray.DataArray, and if not, throw a TypeError.

codecov[bot] commented 3 years ago

Codecov Report

Merging #54 (5d7bfd7) into master (e95d96a) will increase coverage by 0.05%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
+ Coverage   95.74%   95.79%   +0.05%     
==========================================
  Files           3        3              
  Lines         235      238       +3     
  Branches       67       69       +2     
==========================================
+ Hits          225      228       +3     
  Misses          7        7              
  Partials        3        3              
Impacted Files Coverage Ξ”
xhistogram/xarray.py 91.52% <100.00%> (+0.45%) :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 e95d96a...5d7bfd7. Read the comment docs.

dougiesquire commented 3 years ago

Also, it looks as though your additions are failing the pre-commit test. You can run the tests locally to work out the issues by following the instructions here. Let me know if you have any difficulties.

Badboy-16 commented 3 years ago

Thanks @dougiesquire I pushed a new commit regarding the above comments after the pre-commit test is successful locally. Please let me know if there are additional issues :)

dougiesquire commented 3 years ago

Great, pre-commit is passing now πŸ‘. To keep the test coverage high, you could also add a test to xhistogram.tesy.test_xarray.py that checks your input test (sorry, I should have mentioned this sooner). Something like:

def test_input_type_check():
    with pytest.raises(TypeError):
        # Try pass a numpy array to xhistogram
Badboy-16 commented 3 years ago

It's ok. I just added a test in the latest commit. πŸ˜ƒ

dougiesquire commented 3 years ago

Great - all checks passing now, so I'll merge. Thanks for your contribution @Badboy-16!