From 51de49c6aa84d0f22dd2c33446788a0535f0cfc9 Mon Sep 17 00:00:00 2001 From: Noah Biederbeck Date: Wed, 17 Aug 2022 13:59:48 +0200 Subject: [PATCH 1/7] Check table --- pyirf/utils.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pyirf/utils.py b/pyirf/utils.py index a75eaeb98..aec27b06f 100644 --- a/pyirf/utils.py +++ b/pyirf/utils.py @@ -50,6 +50,7 @@ def calculate_theta(events, assumed_source_az, assumed_source_alt): Angular separation between the assumed and reconstructed positions in the sky. """ + check_table(events, required_units={"reco_az": u.deg, "reco_alt": u.deg}) theta = angular_separation( assumed_source_az, assumed_source_alt, @@ -78,6 +79,15 @@ def calculate_source_fov_offset(events, prefix="true"): Angular separation between the true and pointing positions in the sky. """ + check_table( + events, + required_units={ + f"{prefix}_az": u.deg, + f"{prefix}_alt": u.deg, + "pointing_az": u.deg, + "pointing_alt": u.deg, + }, + ) theta = angular_separation( events[f"{prefix}_az"], events[f"{prefix}_alt"], From 0144de2aa89769d862f86a4e59c907015a742ae2 Mon Sep 17 00:00:00 2001 From: Noah Biederbeck Date: Wed, 17 Aug 2022 14:00:12 +0200 Subject: [PATCH 2/7] Check for no unit with a better Error --- pyirf/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyirf/utils.py b/pyirf/utils.py index aec27b06f..2e888824a 100644 --- a/pyirf/utils.py +++ b/pyirf/utils.py @@ -168,5 +168,5 @@ def check_table(table, required_columns=None, required_units=None): raise MissingColumns(col) unit = table[col].unit - if not expected.is_equivalent(unit): + if not unit or not expected.is_equivalent(unit): raise WrongColumnUnit(col, unit, expected) From 631d65877e3666a577e4cca0ef37cde6024f0b4a Mon Sep 17 00:00:00 2001 From: Noah Biederbeck Date: Wed, 17 Aug 2022 14:08:21 +0200 Subject: [PATCH 3/7] Fix WrongColumnUnit text --- pyirf/exceptions.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pyirf/exceptions.py b/pyirf/exceptions.py index 840f384ae..085593183 100644 --- a/pyirf/exceptions.py +++ b/pyirf/exceptions.py @@ -18,7 +18,5 @@ class WrongColumnUnit(IRFException): def __init__(self, column, unit, expected): super().__init__( - f'Unit {unit} of column "{column}"' - f' has incompatible unit "{unit}", expected {expected}' - f" required column {column}" + f'Column "{column}" has incompatible unit "{unit}", expected "{expected}".' ) From 79c928ad954d011791e919fb7874cde33cce3768 Mon Sep 17 00:00:00 2001 From: Noah Biederbeck Date: Wed, 17 Aug 2022 14:42:42 +0200 Subject: [PATCH 4/7] Add test for Table --- pyirf/tests/test_utils.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pyirf/tests/test_utils.py b/pyirf/tests/test_utils.py index 83480ab2d..50d348446 100644 --- a/pyirf/tests/test_utils.py +++ b/pyirf/tests/test_utils.py @@ -33,6 +33,7 @@ def test_cone_solid_angle(): def test_check_table(): from pyirf.exceptions import MissingColumns, WrongColumnUnit from pyirf.utils import check_table + from astropy.table import Table t = QTable({'bar': [0, 1, 2] * u.TeV}) @@ -49,3 +50,7 @@ def test_check_table(): # m is convertible check_table(t, required_units={'bar': u.cm}) + + t = Table({'bar': [0, 1, 2]}) + with pytest.raises(WrongColumnUnit): + check_table(t, required_units={'bar': u.cm}) From 3c5d210abd6dd3079b73d825e050e7566935420a Mon Sep 17 00:00:00 2001 From: Noah Biederbeck Date: Wed, 17 Aug 2022 15:18:07 +0200 Subject: [PATCH 5/7] Add missing tests --- pyirf/tests/test_utils.py | 57 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/pyirf/tests/test_utils.py b/pyirf/tests/test_utils.py index 50d348446..64cbf84b1 100644 --- a/pyirf/tests/test_utils.py +++ b/pyirf/tests/test_utils.py @@ -1,7 +1,8 @@ import numpy as np import astropy.units as u -from astropy.table import QTable +from astropy.table import QTable, Table import pytest +from pyirf.exceptions import MissingColumns, WrongColumnUnit def test_is_scalar(): @@ -31,9 +32,7 @@ def test_cone_solid_angle(): def test_check_table(): - from pyirf.exceptions import MissingColumns, WrongColumnUnit from pyirf.utils import check_table - from astropy.table import Table t = QTable({'bar': [0, 1, 2] * u.TeV}) @@ -54,3 +53,55 @@ def test_check_table(): t = Table({'bar': [0, 1, 2]}) with pytest.raises(WrongColumnUnit): check_table(t, required_units={'bar': u.cm}) + +def test_calculate_theta(): + from pyirf.utils import calculate_theta + + true_az = true_alt = u.Quantity([1.0], u.deg) + t = QTable({'reco_alt': true_alt, 'reco_az': true_az}) + + assert u.isclose( + calculate_theta( + events=t, + assumed_source_az=true_az, + assumed_source_alt=true_alt, + ), + 0.0 * u.deg, + ) + + t = Table({'reco_alt': [1.0], 'reco_az': [1.0]}) + with pytest.raises(WrongColumnUnit): + calculate_theta(t, true_az, true_alt) + +def test_calculate_source_fov_offset(): + from pyirf.utils import calculate_source_fov_offset + + a = u.Quantity([1.0], u.deg) + t = QTable({ + 'pointing_az': a, + 'pointing_alt': a, + 'true_az': a, + 'true_alt': a, + }) + + assert u.isclose(calculate_source_fov_offset(t), 0.0 * u.deg) + +def test_check_histograms(): + from pyirf.binning import create_histogram_table + from pyirf.utils import check_histograms + + events1 = QTable({ + 'reco_energy': [1, 1, 10, 100, 100, 100] * u.TeV, + }) + events2 = QTable({ + 'reco_energy': [100, 100, 100] * u.TeV, + }) + bins = [0.5, 5, 50, 500] * u.TeV + + hist1 = create_histogram_table(events1, bins) + hist2 = create_histogram_table(events2, bins) + check_histograms(hist1, hist2) + + hist3 = create_histogram_table(events1, [0, 10] * u.TeV) + with pytest.raises(ValueError): + check_histograms(hist1, hist3) From abf06f75f8d8720a9d5fa788c64dbae0b832d07e Mon Sep 17 00:00:00 2001 From: Noah Biederbeck Date: Mon, 24 Oct 2022 10:38:07 +0200 Subject: [PATCH 6/7] Explicitly test exception --- pyirf/tests/test_utils.py | 56 ++++++++++++++++++++++++++------------- pyirf/utils.py | 4 +-- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/pyirf/tests/test_utils.py b/pyirf/tests/test_utils.py index 64cbf84b1..78c26ee79 100644 --- a/pyirf/tests/test_utils.py +++ b/pyirf/tests/test_utils.py @@ -1,7 +1,8 @@ -import numpy as np import astropy.units as u -from astropy.table import QTable, Table +import numpy as np import pytest +from astropy.table import QTable, Table + from pyirf.exceptions import MissingColumns, WrongColumnUnit @@ -34,31 +35,45 @@ def test_cone_solid_angle(): def test_check_table(): from pyirf.utils import check_table - t = QTable({'bar': [0, 1, 2] * u.TeV}) - with pytest.raises(MissingColumns): - check_table(t, required_columns=['foo']) + t = Table({"bar": [0, 1, 2] * u.TeV}) - t = QTable({'bar': [0, 1, 2] * u.TeV}) - with pytest.raises(WrongColumnUnit): - check_table(t, required_units={'bar': u.m}) + with pytest.raises( + MissingColumns, + match="Table is missing required columns {'foo'}", + ): + check_table(t, required_columns=["foo"]) - t = QTable({'bar': [0, 1, 2] * u.m}) - with pytest.raises(MissingColumns): - check_table(t, required_units={'foo': u.m}) + t = Table({"bar": [0, 1, 2] * u.TeV}) + with pytest.raises( + WrongColumnUnit, + match='Column "bar" has incompatible unit "TeV", expected "m".', + ): + check_table(t, required_units={"bar": u.m}) + + t = Table({"bar": [0, 1, 2] * u.m}) + with pytest.raises( + MissingColumns, + match="Table is missing required columns foo", + ): + check_table(t, required_units={"foo": u.m}) # m is convertible - check_table(t, required_units={'bar': u.cm}) + check_table(t, required_units={"bar": u.cm}) + + t = Table({"bar": [0, 1, 2]}) + with pytest.raises( + WrongColumnUnit, + match='Column "bar" has incompatible unit "None", expected "cm".', + ): + check_table(t, required_units={"bar": u.cm}) - t = Table({'bar': [0, 1, 2]}) - with pytest.raises(WrongColumnUnit): - check_table(t, required_units={'bar': u.cm}) def test_calculate_theta(): from pyirf.utils import calculate_theta true_az = true_alt = u.Quantity([1.0], u.deg) - t = QTable({'reco_alt': true_alt, 'reco_az': true_az}) + t = QTable({"reco_alt": true_alt, "reco_az": true_az}) assert u.isclose( calculate_theta( @@ -69,10 +84,14 @@ def test_calculate_theta(): 0.0 * u.deg, ) - t = Table({'reco_alt': [1.0], 'reco_az': [1.0]}) - with pytest.raises(WrongColumnUnit): + t = Table({"reco_alt": [1.0], "reco_az": [1.0]}) + with pytest.raises( + WrongColumnUnit, + match='Column "reco_az" has incompatible unit "None", expected "deg".', + ): calculate_theta(t, true_az, true_alt) + def test_calculate_source_fov_offset(): from pyirf.utils import calculate_source_fov_offset @@ -86,6 +105,7 @@ def test_calculate_source_fov_offset(): assert u.isclose(calculate_source_fov_offset(t), 0.0 * u.deg) + def test_check_histograms(): from pyirf.binning import create_histogram_table from pyirf.utils import check_histograms diff --git a/pyirf/utils.py b/pyirf/utils.py index 2e888824a..f3a865a80 100644 --- a/pyirf/utils.py +++ b/pyirf/utils.py @@ -1,10 +1,10 @@ -import numpy as np import astropy.units as u +import numpy as np from astropy.coordinates.angle_utilities import angular_separation +from astropy.table import QTable from .exceptions import MissingColumns, WrongColumnUnit - __all__ = [ "is_scalar", "calculate_theta", From 6ae339569fdfb3782e8e855bd3aa3a0addd98dce Mon Sep 17 00:00:00 2001 From: Noah Biederbeck Date: Mon, 24 Oct 2022 10:38:40 +0200 Subject: [PATCH 7/7] Convert to QTable, to work as well with Table --- pyirf/tests/test_utils.py | 34 ++++++++++++++++++++++------------ pyirf/utils.py | 3 ++- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/pyirf/tests/test_utils.py b/pyirf/tests/test_utils.py index 78c26ee79..d0677b62e 100644 --- a/pyirf/tests/test_utils.py +++ b/pyirf/tests/test_utils.py @@ -35,6 +35,10 @@ def test_cone_solid_angle(): def test_check_table(): from pyirf.utils import check_table + # works with Table as well as QTable + check_table(Table({"foo": [1]}), required_columns=["foo"]) + check_table(Table({"foo": [1] * u.m}), required_units={"foo": u.m}) + check_table(QTable({"foo": [1] * u.m}), required_units={"foo": u.m}) t = Table({"bar": [0, 1, 2] * u.TeV}) @@ -96,12 +100,14 @@ def test_calculate_source_fov_offset(): from pyirf.utils import calculate_source_fov_offset a = u.Quantity([1.0], u.deg) - t = QTable({ - 'pointing_az': a, - 'pointing_alt': a, - 'true_az': a, - 'true_alt': a, - }) + t = QTable( + { + "pointing_az": a, + "pointing_alt": a, + "true_az": a, + "true_alt": a, + } + ) assert u.isclose(calculate_source_fov_offset(t), 0.0 * u.deg) @@ -110,12 +116,16 @@ def test_check_histograms(): from pyirf.binning import create_histogram_table from pyirf.utils import check_histograms - events1 = QTable({ - 'reco_energy': [1, 1, 10, 100, 100, 100] * u.TeV, - }) - events2 = QTable({ - 'reco_energy': [100, 100, 100] * u.TeV, - }) + events1 = QTable( + { + "reco_energy": [1, 1, 10, 100, 100, 100] * u.TeV, + } + ) + events2 = QTable( + { + "reco_energy": [100, 100, 100] * u.TeV, + } + ) bins = [0.5, 5, 50, 500] * u.TeV hist1 = create_histogram_table(events1, bins) diff --git a/pyirf/utils.py b/pyirf/utils.py index f3a865a80..26c37b06f 100644 --- a/pyirf/utils.py +++ b/pyirf/utils.py @@ -143,7 +143,7 @@ def check_table(table, required_columns=None, required_units=None): Parameters ---------- - table: astropy.table.QTable + table: astropy.table.Table Table to check required_columns: iterable[str] Column names that are required to be present @@ -157,6 +157,7 @@ def check_table(table, required_columns=None, required_units=None): as keys in ``required_units are`` not present in the table. WrongColumnUnit: if any column has the wrong unit """ + table = QTable(table) if required_columns is not None: missing = set(required_columns) - set(table.colnames) if missing: