Bug in the commit 8a52f3dc3fb695094f678cf5e36b7f97b99a0f31

[giulianoiorio]

The commit
commit 8a52f3d (HEAD)
Author: GNicola [email protected]
Date: Mon Aug 2 09:29:35 2021 +0200

total mass to test

introduced some kind of bug in the module.

It seems that the generated masses are completely off with respect to the expected distribution.
In particular, the masses are all very small (around 0.1 Msun), this is confirmed also by the plots generated by pytest (below).

I confirm that the previous commit works as expected

[GiacobboNicola]

I fixed the bug (commit ca75776) for the plot (below) but still some issue with the second test:
chi, _ = chisquare(hist, y)
the scipy.stats.chisquare has been updated (and there is a know problem). Did you perform the test with both commits on the same machine?

[giulianoiorio]

Thank you Nicola,
I see the problem, it is just that mass_range is now called mass_ranges, notice that the example.py should be also corrected for this. This is very important since this file is "the entry point" for new module users. Indeed I found this problem helping a student that was trying to generate sevn ics following what written in example.py. I am pushing this update with a new commit.
Concerning the other tests, they were successful on my office machine (scipy 1.6.2) while they fail in both commits on the student laptop (I guess his scipy version is more recent than min since it just recently installed python). I confirm that with this scipy version all the tests succeed also for the new commit.

Concerning the reason of the bug (change of name of a parameter), I think for the future we should use the practice to allow for the first new commits to use both the old and the parameters name raising a DepreactedWarning if the old names are used.

[GiacobboNicola]

Thank for updating the file. You are right, I completely forgot to check example.py.
I will look for a possible solution in the following days.

I agree with the idea of raising Deprecated/Warning for old names.

[giulianoiorio]

I think we can close the issue :)