From b5f9ca17095119323472704d10e799e4742c1ab3 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Thu, 29 Aug 2024 18:52:02 +0530 Subject: [PATCH 01/16] Event_folding key validation --- stingray/pulse/pulsar.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index 4e84ab8bd..7dd05015c 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -43,6 +43,17 @@ def _default_value_if_no_key(dictionary, key, default): except: return default +def validate_key_in_list(key, optional_keys): + + ''' + if key not in optional_keys: + raise ValueError(f"The key '{key}' is not found in the list of optional keys: {optional_keys}") + + ''' + + if key not in optional_keys: + warnings.warn(f"The key '{key}' is not in the list of optional keys: {optional_keys}", UserWarning) + return key in optional_keys def p_to_f(*period_derivatives): """Convert periods into frequencies, and vice versa. @@ -263,6 +274,19 @@ def fold_events(times, *frequency_derivatives, **opts): profile_err : array of floats The uncertainties on the pulse profile """ + + optional_parameters = [ + 'nbins', + 'weights', + 'gti', + 'ref_time', + 'expocorr', + 'mode' + ] + + for key in opts: + validate_key_in_list(key, optional_keys= optional_parameters) + 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) From d6ef817b5970285b5a2b8369a68eb604c64375b0 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Thu, 29 Aug 2024 23:47:18 +0530 Subject: [PATCH 02/16] Formatting --- stingray/pulse/pulsar.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index 7dd05015c..9e9f1345b 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -43,18 +43,21 @@ def _default_value_if_no_key(dictionary, key, default): except: return default -def validate_key_in_list(key, optional_keys): - ''' +def validate_key_in_list(key, optional_keys): + """ if key not in optional_keys: raise ValueError(f"The key '{key}' is not found in the list of optional keys: {optional_keys}") - ''' + """ if key not in optional_keys: - warnings.warn(f"The key '{key}' is not in the list of optional keys: {optional_keys}", UserWarning) + warnings.warn( + f"The key '{key}' is not in the list of optional keys: {optional_keys}", UserWarning + ) return key in optional_keys + def p_to_f(*period_derivatives): """Convert periods into frequencies, and vice versa. @@ -275,17 +278,10 @@ def fold_events(times, *frequency_derivatives, **opts): The uncertainties on the pulse profile """ - optional_parameters = [ - 'nbins', - 'weights', - 'gti', - 'ref_time', - 'expocorr', - 'mode' - ] + optional_parameters = ["nbins", "weights", "gti", "ref_time", "expocorr", "mode"] for key in opts: - validate_key_in_list(key, optional_keys= optional_parameters) + validate_key_in_list(key, optional_keys=optional_parameters) mode = _default_value_if_no_key(opts, "mode", "ef") nbin = _default_value_if_no_key(opts, "nbin", 16) From 13646d86226cbb5915027cdbd9f8f17cf6f95be8 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Fri, 30 Aug 2024 18:54:45 +0530 Subject: [PATCH 03/16] Attempting to use dict.pop() --- stingray/pulse/pulsar.py | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index 9e9f1345b..a868b30a0 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -44,20 +44,6 @@ def _default_value_if_no_key(dictionary, key, default): return default -def validate_key_in_list(key, optional_keys): - """ - if key not in optional_keys: - raise ValueError(f"The key '{key}' is not found in the list of optional keys: {optional_keys}") - - """ - - if key not in optional_keys: - warnings.warn( - f"The key '{key}' is not in the list of optional keys: {optional_keys}", UserWarning - ) - return key in optional_keys - - def p_to_f(*period_derivatives): """Convert periods into frequencies, and vice versa. @@ -127,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 = dict.pop(opts, "ph0", 0) + to_1 = dict.pop(opts, "to_1", True) ph = ph0 for i_f, f in enumerate(frequency_derivatives): @@ -278,23 +264,23 @@ def fold_events(times, *frequency_derivatives, **opts): The uncertainties on the pulse profile """ - optional_parameters = ["nbins", "weights", "gti", "ref_time", "expocorr", "mode"] - - for key in opts: - validate_key_in_list(key, optional_keys=optional_parameters) - - 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 = dict.pop(opts, "mode", "ef") + nbin = dict.pop(opts, "nbin", 16) + weights = dict.pop(opts, "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 = dict.pop(opts, "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 = dict.pop(opts, "ref_time", 0) + expocorr = dict.pop(opts, "expocorr", False) + + if opts: + raise ValueError(f"Unidentified keys: {opts}") + + print(mode, nbin, weights) if not isinstance(weights, Iterable): weights *= np.ones(len(times)) From 638afece66276a3b12c68cdf6fb2d3eb65c5df89 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Fri, 30 Aug 2024 19:06:26 +0530 Subject: [PATCH 04/16] Formatting 2.0 --- stingray/pulse/pulsar.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index a868b30a0..a60ef7d83 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -278,9 +278,9 @@ def fold_events(times, *frequency_derivatives, **opts): expocorr = dict.pop(opts, "expocorr", False) if opts: - raise ValueError(f"Unidentified keys: {opts}") - - print(mode, nbin, weights) + raise ValueError( + f"Unidentified keys: {opts.keys()} \n Please refer to the docstring for optional parameters." + ) if not isinstance(weights, Iterable): weights *= np.ones(len(times)) From d5806c43cba8369a7d6b83ba7ee9bf4f2c383aab Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Fri, 30 Aug 2024 19:17:01 +0530 Subject: [PATCH 05/16] Editted value error message --- stingray/pulse/pulsar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index a60ef7d83..490986235 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -279,7 +279,7 @@ def fold_events(times, *frequency_derivatives, **opts): if opts: raise ValueError( - f"Unidentified keys: {opts.keys()} \n Please refer to the docstring for optional parameters." + f"Unidentified keys: {opts.keys()} \n Please refer to the description of the function for optional parameters." ) if not isinstance(weights, Iterable): From 470ea84ca67e42ad15d4c7e69f585b312107182a Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Mon, 2 Sep 2024 18:04:40 +0530 Subject: [PATCH 06/16] opts.pop instead of dict.pop --- stingray/pulse/pulsar.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index 490986235..61270a537 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -113,8 +113,8 @@ def pulse_phase(times, *frequency_derivatives, **opts): """ - ph0 = dict.pop(opts, "ph0", 0) - to_1 = dict.pop(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): @@ -264,18 +264,18 @@ def fold_events(times, *frequency_derivatives, **opts): The uncertainties on the pulse profile """ - mode = dict.pop(opts, "mode", "ef") - nbin = dict.pop(opts, "nbin", 16) - weights = dict.pop(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 = dict.pop(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 = dict.pop(opts, "ref_time", 0) - expocorr = dict.pop(opts, "expocorr", False) + ref_time = opts.pop( "ref_time", 0) + expocorr = opts.pop( "expocorr", False) if opts: raise ValueError( From 5fa9667a84e63b5563bd4ac8386439d4be64c503 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Mon, 2 Sep 2024 18:13:24 +0530 Subject: [PATCH 07/16] Formating 3.0 --- stingray/pulse/pulsar.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index 61270a537..723cb8fa9 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -113,8 +113,8 @@ def pulse_phase(times, *frequency_derivatives, **opts): """ - ph0 = opts.pop( "ph0", 0) - to_1 = opts.pop( "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): @@ -264,18 +264,18 @@ def fold_events(times, *frequency_derivatives, **opts): The uncertainties on the pulse profile """ - mode = opts.pop( "mode", "ef") - nbin = opts.pop( "nbin", 16) - weights = opts.pop( "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 = opts.pop( "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 = opts.pop( "ref_time", 0) - expocorr = opts.pop( "expocorr", False) + ref_time = opts.pop("ref_time", 0) + expocorr = opts.pop("expocorr", False) if opts: raise ValueError( From 327e9eb679242f0e2ecdb2762d08ad3736787dc1 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Fri, 13 Sep 2024 11:38:30 +0530 Subject: [PATCH 08/16] Changelog updated --- docs/changes/837.bugfix.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 docs/changes/837.bugfix.rst diff --git a/docs/changes/837.bugfix.rst b/docs/changes/837.bugfix.rst new file mode 100644 index 000000000..bdebe104b --- /dev/null +++ b/docs/changes/837.bugfix.rst @@ -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. From 68a2931eb5b611fe82c96336f5e94219a7a22329 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Fri, 13 Sep 2024 19:32:04 +0530 Subject: [PATCH 09/16] Removing ax as a kwarg from test cases: test_plot_profile_errorbars, test_plot_profile_existing_ax --- stingray/pulse/tests/test_search.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index 749cd1934..f002436d1 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -120,7 +120,7 @@ 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) @@ -128,7 +128,7 @@ def test_plot_profile_existing_ax(self): 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") From dff319a5dbd3197b366f55e0217d8e7162330882 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Wed, 18 Sep 2024 18:45:19 +0530 Subject: [PATCH 10/16] Updating error message and adding test case --- stingray/pulse/pulsar.py | 2 +- stingray/pulse/tests/test_search.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index 723cb8fa9..a8840a68a 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -279,7 +279,7 @@ def fold_events(times, *frequency_derivatives, **opts): if opts: raise ValueError( - f"Unidentified keys: {opts.keys()} \n Please refer to the description of the function for optional parameters." + f"Unidentified keyword to fold_events: {opts.keys()} \n Please refer to the description of the function for optional parameters." ) if not isinstance(weights, Iterable): diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index f002436d1..3ca294360 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -111,6 +111,12 @@ def test_plot_phaseogram_direct(self): plt.savefig("phaseogram_direct.png") plt.close(plt.gcf()) + def test_search_wrongk_key_fails(self): + with pytest.raises( + ValueError, match="Unidentified keyword to fold_events: dict_keys(['fdot'])" + ) as excinfo: + phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12) + def test_plot_profile(self): phase, prof, _ = fold_events(self.event_times, self.pulse_frequency) ax = plot_profile(phase, prof) From d6af58452952b57ce9b4cee7f9b3fdd0f0169a09 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Wed, 18 Sep 2024 18:52:22 +0530 Subject: [PATCH 11/16] fixing typo --- stingray/pulse/pulsar.py | 2 +- stingray/pulse/tests/test_search.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index a8840a68a..904e92980 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -279,7 +279,7 @@ def fold_events(times, *frequency_derivatives, **opts): if opts: raise ValueError( - f"Unidentified keyword to fold_events: {opts.keys()} \n Please refer to the description of the function for optional parameters." + f"Unidentified keyword(s) to fold_events: {opts.keys()} \n Please refer to the description of the function for optional parameters." ) if not isinstance(weights, Iterable): diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index 3ca294360..77f6e7a33 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -111,10 +111,10 @@ def test_plot_phaseogram_direct(self): plt.savefig("phaseogram_direct.png") plt.close(plt.gcf()) - def test_search_wrongk_key_fails(self): + def test_search_wrong_key_fails(self): with pytest.raises( - ValueError, match="Unidentified keyword to fold_events: dict_keys(['fdot'])" - ) as excinfo: + ValueError, match="Unidentified keyword(s) to fold_events: dict_keys(['fdot'])" + ): phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12) def test_plot_profile(self): From d7603765796376396a5ffab585598ff8a0ebf64a Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Wed, 18 Sep 2024 21:44:02 +0530 Subject: [PATCH 12/16] Final Changes --- stingray/pulse/pulsar.py | 2 +- stingray/pulse/tests/test_search.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/stingray/pulse/pulsar.py b/stingray/pulse/pulsar.py index 904e92980..fce5abc41 100644 --- a/stingray/pulse/pulsar.py +++ b/stingray/pulse/pulsar.py @@ -279,7 +279,7 @@ def fold_events(times, *frequency_derivatives, **opts): if opts: raise ValueError( - f"Unidentified keyword(s) to fold_events: {opts.keys()} \n Please refer to the description of the function for optional parameters." + 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): diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index 77f6e7a33..e10ef96f2 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -113,9 +113,9 @@ def test_plot_phaseogram_direct(self): def test_search_wrong_key_fails(self): with pytest.raises( - ValueError, match="Unidentified keyword(s) to fold_events: dict_keys(['fdot'])" + ValueError, match="Unidentified keyword(s) to fold_events: fdot, fddot" ): - phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12) + 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) From 035a77d9879bbd002520904c4523efeb9951bdc5 Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Wed, 18 Sep 2024 22:09:43 +0530 Subject: [PATCH 13/16] Formatting --- stingray/pulse/tests/test_search.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index e10ef96f2..e8a9c9f3b 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -112,9 +112,7 @@ def test_plot_phaseogram_direct(self): plt.close(plt.gcf()) def test_search_wrong_key_fails(self): - with pytest.raises( - ValueError, match="Unidentified keyword(s) to fold_events: fdot, fddot" - ): + with pytest.raises(ValueError, match="Unidentified keyword(s) to fold_events: fdot, fddot"): phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12, fddot=34) def test_plot_profile(self): From a40ebfba8645d2a607fac7aff47e59e1e58e9eef Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Thu, 19 Sep 2024 00:50:56 +0530 Subject: [PATCH 14/16] Resolving issue --- stingray/pulse/tests/test_search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index e8a9c9f3b..bf0556640 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -112,7 +112,7 @@ def test_plot_phaseogram_direct(self): plt.close(plt.gcf()) def test_search_wrong_key_fails(self): - with pytest.raises(ValueError, match="Unidentified keyword(s) to fold_events: fdot, fddot"): + 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."): phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12, fddot=34) def test_plot_profile(self): From efefb48181d2bbd0bfe178352c6459a38096b56a Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Thu, 19 Sep 2024 00:52:53 +0530 Subject: [PATCH 15/16] Formatting --- stingray/pulse/tests/test_search.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index bf0556640..293175ff5 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -112,7 +112,10 @@ def test_plot_phaseogram_direct(self): 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."): + 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.", + ): phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12, fddot=34) def test_plot_profile(self): From e5f0653e76934e20274af2f14363789d87b5ba1e Mon Sep 17 00:00:00 2001 From: spranav1205 Date: Thu, 19 Sep 2024 18:38:36 +0530 Subject: [PATCH 16/16] Regex error fixed --- stingray/pulse/tests/test_search.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/stingray/pulse/tests/test_search.py b/stingray/pulse/tests/test_search.py index 293175ff5..1e35071e5 100644 --- a/stingray/pulse/tests/test_search.py +++ b/stingray/pulse/tests/test_search.py @@ -113,8 +113,7 @@ def test_plot_phaseogram_direct(self): 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.", + ValueError, match=r"Unidentified keyword\(s\) to fold_events: fdot, fddot" ): phase, prof, _ = fold_events(self.event_times, self.pulse_frequency, fdot=12, fddot=34)