Fix err dist none

[andresgur]

Fixes #908

Provide an overview of the implemented solution or the fix and elaborate on the modifications.

I have modified the valid_statistics in lightcurve.py. Instead of the using the old None, now we use "None".

valid_statistics = ["poisson", "gauss", "None"]

I have also changed the default err_dist in the Lightcurve constructor to "None" to correctly reflect the docs.

from stingray import Lightcurve
import numpy as np
from stingray.lightcurve import valid_statistics


times = np.arange(1000)

mean_flux = 100.0
std_flux = 2.0 
flux = np.random.normal(loc=mean_flux, scale=std_flux, size=len(times))
flux_err = np.ones_like(flux) * std_flux

print(valid_statistics)

lc = Lightcurve(times, flux, err=flux_err, err_dist="None", dt=1.0, skip_checks=True)
np.testing.assert_array_equal(lc.counts_err, np.full(lc.n, std_flux))


lc = Lightcurve(times, flux, err=None, err_dist="None", dt=1.0, skip_checks=True)
np.testing.assert_array_equal(lc.counts_err, np.zeros(lc.n))

[matteobachetti]

@andresgur Thanks for your PR. I made a slight modification to avoid breaking the API.

[andresgur]

thanks @matteobachetti ! Is there anything else for me to do? Sorry still learning this whole PR thing

[matteobachetti]

thanks @matteobachetti ! Is there anything else for me to do? Sorry still learning this whole PR thing

No worries! I'm taking care of solving the API change issues. Please write a changelog snippet according to https://docs.stingray.science/en/stable/contributing.html#updating-and-maintaining-the-changelog. It should be called docs/changes/909.bugfix.rst and briefly describe the changes

[ankitkhushwaha]

Hey @andresgur , i didn't see your work here and fixes err_dist issue in #912 . But i think it is good to support the None object not the str one. Users always more tends to use None (object) not "none".

[matteobachetti]

@andresgur the change of default means that a warning is thrown everywhere in the code now during tests, making about 50 tests fail. I think you found an important inconsistency in the docs, and defaulting to "none" is indeed more correct, but this also means that we need to change a lot of tests, catching the warning with constructs like

with pytest.warns(UserWarning, match="Beware! Stingray only uses"):
     <etc.>

[andresgur]

Added the file @matteobachetti

[ankitkhushwaha]

the change of default means that a warning is thrown everywhere in the code now during tests, making about 50 tests fail. I think you found an important inconsistency in the docs, and defaulting to "none" is indeed more correct, but this also means that we need to change a lot of tests, catching the warning with constructs like

with pytest.warns(UserWarning, match="Beware! Stingray only uses"):
     <etc.>

Hey @andresgur, we also need to fix the tests raising above warning.

[andresgur]

which ones? No tests break for me... I thought the problem was keeping none as a string

[ankitkhushwaha]

which ones? No tests break for me... I thought the problem was keeping none as a string

did u run pytest in your machine? Api is returning new warning, check once.

[andresgur]

for file in test*.py ; do echo $file; python $file -m unitest; done

test_base.py test_bexvar.py test_bispectrum.py test_covariancespectrum.py test_crosscorrelation.py test_crossspectrum.py test_events.py Traceback (most recent call last): File "/home/andresgur/scripts/pythonscripts/stingray/stingray/tests/test_events.py", line 8, in from ..events import EventList ImportError: attempted relative import with no known parent package test_filters.py Traceback (most recent call last): File "/home/andresgur/scripts/pythonscripts/stingray/stingray/tests/test_filters.py", line 6, in from ..filters import Window1D, Optimal1D, filter_for_deadtime ImportError: attempted relative import with no known parent package test_fourier.py test_gti.py test_io.py Traceback (most recent call last): File "/home/andresgur/scripts/pythonscripts/stingray/stingray/tests/test_io.py", line 8, in from ..io import split_numbers ImportError: attempted relative import with no known parent package test_lightcurve.py test_lombscargle.py test_multitaper.py test_power_colors.py test_powerspectrum.py test_sampledata.py Traceback (most recent call last): File "/home/andresgur/scripts/pythonscripts/stingray/stingray/tests/test_sampledata.py", line 1, in from ..sampledata import sample_data ImportError: attempted relative import with no known parent package test_spectroscopy.py test_stats.py test_utils.py test_varenergyspectrum.py

Am I missing something?

[ankitkhushwaha]

Hey @andresgur, for stingray we uses pytest for testing

inside the stingray repository run pytest it will show the test results

u may refer this https://docs.stingray.science/en/stable/#test-suite

[andresgur]

thanks I see it now... so we should skip the warnings as @matteobachetti suggested?

[ankitkhushwaha]

thanks I see it now... so we should skip the warnings as @matteobachetti suggested?

yes,
You need to replace the text "simon ..." with "Beware! Stingray only uses". same as suggested by @ matteobachetti

[matteobachetti]

@andresgur tests are run using pytest or, better, tox -e test. In any of the failed test suites above, you will find about 50 tests that failed. We should look at each of them and, if the Poisson stats was an adequate default (the light curve is being formed by integer numbers of counts), we should add err_dist="poisson" to the Lightcurve initialization. Otherwise, the line that throws the warning should be put inside the construct I made above.

[andresgur]

All tests passing on my end now