search

icecube / pisa

Monte Carlo-based data analysis
http://icecube.github.io/pisa/
Apache License 2.0
19 stars 47 forks source link

Add missing method to assert compatibility #701

Closed atrettin closed 2 years ago

atrettin commented 2 years ago

The assert_compat method was missing from the OneDimBinning class, and therefore the assert_compat method from the MultiDimBinning class would fail with an obscure error. This PR fixes this behavior and adds a unit test to ensure that the appropriate error is raised.

atrettin commented 2 years ago

@philippeller The test is failing because the pytest package is not installed by default. I think that pytest is definitely an essential thing to have for a developer, but it may not be necessary for a use who isn't intending to run unit tests. Would it make sense to add pytest to the list of required packages for develop installs, and then also to change the GitHub workflow such that PISA is installed in developer mode for testing? Or is that too convoluted, we could also just add pytest to the list of packages that are always required and be done with it.

philippeller commented 2 years ago

Yes, i think that would be the preferred solution 🙌

atrettin commented 2 years ago

Yes, i think that would be the preferred solution 🙌

Which one? 😅

atrettin commented 2 years ago

This should work! Shall we merge?

philippeller commented 2 years ago

back from vacation! Yes, I'll merge this