Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fold_events keyword validation #837

Merged
merged 16 commits into from
Sep 19, 2024
3 changes: 3 additions & 0 deletions docs/changes/837.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
The ``fold_events`` function now checks if the keyword arguments (`kwargs`) are in the list of optional parameters.
If any unidentified keys are present, it raises a `ValueError`.
This fix ensures that the function only accepts valid optional parameters and provides a clear error message for unsupported keys.
22 changes: 14 additions & 8 deletions stingray/pulse/pulsar.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ def pulse_phase(times, *frequency_derivatives, **opts):

"""

ph0 = _default_value_if_no_key(opts, "ph0", 0)
to_1 = _default_value_if_no_key(opts, "to_1", True)
ph0 = opts.pop("ph0", 0)
to_1 = opts.pop("to_1", True)
ph = ph0

for i_f, f in enumerate(frequency_derivatives):
Expand Down Expand Up @@ -263,18 +263,24 @@ def fold_events(times, *frequency_derivatives, **opts):
profile_err : array of floats
The uncertainties on the pulse profile
"""
mode = _default_value_if_no_key(opts, "mode", "ef")
nbin = _default_value_if_no_key(opts, "nbin", 16)
weights = _default_value_if_no_key(opts, "weights", 1)

mode = opts.pop("mode", "ef")
nbin = opts.pop("nbin", 16)
weights = opts.pop("weights", 1)
# If no key is passed, *or gti is None*, defaults to the
# initial and final event
gti = _default_value_if_no_key(opts, "gti", None)
gti = opts.pop("gti", None)
if gti is None:
gti = [[times[0], times[-1]]]
# Be safe if gtis are a list
gti = np.asanyarray(gti)
ref_time = _default_value_if_no_key(opts, "ref_time", 0)
expocorr = _default_value_if_no_key(opts, "expocorr", False)
ref_time = opts.pop("ref_time", 0)
expocorr = opts.pop("expocorr", False)

if opts:
raise ValueError(
f"Unidentified keyword(s) to fold_events: {', '.join([k for k in opts.keys()])} \n Please refer to the description of the function for optional parameters."
)

if not isinstance(weights, Iterable):
weights *= np.ones(len(times))
Expand Down
11 changes: 9 additions & 2 deletions stingray/pulse/tests/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ def test_plot_phaseogram_direct(self):
plt.savefig("phaseogram_direct.png")
plt.close(plt.gcf())

def test_search_wrong_key_fails(self):
with pytest.raises(
ValueError,
match="Unidentified keyword(s) to fold_events: fdot, fddot \n Please refer to the description of the function for optional parameters.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
match="Unidentified keyword(s) to fold_events: fdot, fddot \n Please refer to the description of the function for optional parameters.",
match="Unidentified keyword(s) to fold_events: fdot, fddot",

Its better to match the smallest possible part of the regex that makes it really specific. Try to run it locally in your computer, with the --run-slow option, to verify that it works

Copy link
Contributor Author

@spranav1205 spranav1205 Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The brackets were causing that issue. I fixed it and the test is working on my PC.
Two checks are failing. It says the tests do not cover line 281.
Line 281 is raise ValueError(. But the test case covers it so I am not sure why the check is failing.

):
phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12, fddot=34)

def test_plot_profile(self):
phase, prof, _ = fold_events(self.event_times, self.pulse_frequency)
ax = plot_profile(phase, prof)
Expand All @@ -120,15 +127,15 @@ def test_plot_profile(self):
def test_plot_profile_existing_ax(self):
fig = plt.figure("Pulse profile")
ax = plt.subplot()
phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, ax=ax)
phase, prof, _ = fold_events(self.event_times, self.pulse_frequency)
ax = plot_profile(phase, prof, ax=ax)
plt.savefig("profile_existing_ax.png")
plt.close(fig)

def test_plot_profile_errorbars(self):
fig = plt.figure("Pulse profile")
ax = plt.subplot()
phase, prof, err = fold_events(self.event_times, self.pulse_frequency, ax=ax)
phase, prof, err = fold_events(self.event_times, self.pulse_frequency)

ax = plot_profile(phase, prof, err=err, ax=ax)
plt.savefig("profile_errorbars.png")
Expand Down
Loading