Use LSST transmission test data for passband unit tests #135
Conversation
…'re using default dir or not
| table_dir : str, optional | ||
| The path to the directory containing the passband tables. If a table_path has not been specified | ||
| in the passband_parameters dictionary, table paths will be set to | ||
| {table_dir}/{survey}/{filter_name}.dat. If None, the table path will be set to a default path. |
There was a problem hiding this comment.
No change needed right now, but I had been thinking of this as "{table_dir}/{filter_name}.dat" so the user could effectively provide both the base directory and the survey name with the table_dir parameter.
This current approach better matches the presets below though.
src/tdastro/astro_utils/passbands.py
Outdated
| self.passbands[f"LSST_{filter_name}"] = Passband( | ||
| "LSST", | ||
| filter_name, | ||
| **{**kwargs, "table_path": table_path}, |
There was a problem hiding this comment.
I think it would be more readable to just do:
table_path=table_path,
**kwargs,
)
| from tdastro.sources.spline_model import SplineModel | ||
|
|
||
|
|
||
| def create_passband_group(path, delta_wave=5.0, trim_quantile=None): | ||
| """Helper function to create a PassbandGroup object for testing.""" | ||
| def create_lsst_passband_group(passbands_dir, delta_wave=5.0, trim_quantile=None): |
There was a problem hiding this comment.
Looks like a nice function for the library code!
| assert "TOY_c" in toy_passband_group.passbands | ||
| assert np.allclose( | ||
| toy_passband_group.waves, | ||
| np.unique(np.concatenate([np.arange(100, 301, 5), np.arange(250, 351, 5), np.arange(400, 601, 5)])), |
There was a problem hiding this comment.
This looks like the library code duplication.
| # Make sure group.waves contains the union of all passband waves with no duplicates or other values | ||
| assert np.allclose( | ||
| lsst_passband_group.waves, | ||
| np.unique(np.concatenate([passband.waves for passband in lsst_passband_group.passbands.values()])), |
There was a problem hiding this comment.
This also duplicates the library code
|
|
||
| # Check the two dicts are the same | ||
| assert result_from_source_model.keys() == result_from_passband_group.keys() | ||
| for key in result_from_source_model: | ||
| np.testing.assert_allclose(result_from_source_model[key], result_from_passband_group[key]) | ||
|
|
||
|
|
||
| def test_passband_group_calculate_in_band_wave_indices(passbands_dir, tmp_path): |
There was a problem hiding this comment.
I'm actually a bit confused about the original function and different return formats for different cases. If a passband waves are represented by a range of catalog.waves it returns (lower_index, upper_index + 1) tuple, while for the opposite case it is an integer array of indexes. Than here it is used to index an array:
tdastro/src/tdastro/astro_utils/passbands.py
Lines 187 to 190 in 7450fcc
I'd suggest to return slice(lower_index, upper_index + 1) instead, than both types of the return indexes may be just used directly.
Should I make a separate issue for that?
For the first part of #113 (updating unit tests to use LSST transmission tables stored in unit test data).
Should also cover #127 (LSST g band file is wrong).
Introduces a
table_diroption to PassbandGroup, so you can specify a general transmission table directory to use, as in:PassbandGroup(preset="LSST", table_dir="my/dir")PassbandGroup(table_dir="my/dir", passband_parameters=[...]))