V0.6.1+4lst - #329
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #329 +/- ##
==========================================
+ Coverage 75.89% 79.64% +3.75%
==========================================
Files 22 22
Lines 2750 3237 +487
==========================================
+ Hits 2087 2578 +491
+ Misses 663 659 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There are probably some tests not actually needed, but we can start to have a look into that to start pruning the 'tree' of 4 LST PRs |
| telescope_combinations, | ||
| ) | ||
|
|
||
| LOGGER = logging.getLogger(__name__) |
There was a problem hiding this comment.
this LOGGER does not seem to be used anywhere
| @@ -1,11 +1,13 @@ | |||
| import glob | |||
| import logging | |||
There was a problem hiding this comment.
in principle the tests are by default executed with all the text output supressed, do we need this part?
| query_test = pd.read_hdf(query_test_1, key="/events/parameters") | ||
| data = query_data(query_test, "software", None, [], False) | ||
| assert ( | ||
| 'Requested event type and provided telescopes IDs are not consistent; "software" must be used in case of standard MAGIC+LST-1 analyses' |
There was a problem hiding this comment.
now I see where you use the log, but such a test is dangerous: enough that somebody will change slightly the message and the test will fail. This looks like a fatal error, so we could e.g. throw an exception that can be tested here
| query_test = pd.read_hdf(query_test_2, key="/events/parameters") | ||
| data = query_data(query_test, "magic_only", None, [], False) | ||
| assert ( | ||
| "MAGIC-only analysis requested, but inconsistent with the provided telescope IDs: check the configuration file" |
There was a problem hiding this comment.
as above - better to e.g. throw an exception that is tested here
| data = query_data(query_test, "magic_only", 3, [], False) | ||
| assert np.allclose(np.array(data["event_id"]), np.array([2, 5, 8])) | ||
|
|
||
| query_test = pd.read_hdf(query_test_2, key="/events/parameters") |
There was a problem hiding this comment.
since there are many lines like this it can be passed as parametrized test or just a loop with input/output values listed in an array/dictionaries
| data = query_data(query_test, "trigger_3tels_or_more", None, [4, 5, 6], False) | ||
| assert np.allclose(np.array(data["event_id"]), np.array([1, 3, 9])) | ||
|
|
||
| query_test = pd.read_hdf(query_test_2, key="/events/parameters") |
There was a problem hiding this comment.
this line (and e.g. in L65) do not seem needed for me since query_test is not expected to change from L55
| "LST-1_MAGIC-II": [1, 3], # combo_type = 2 | ||
| "MAGIC-I_MAGIC-II": [2, 3], # combo_type = 3 | ||
| } | ||
| assert list(M_LST_comb.keys()) == [ |
There was a problem hiding this comment.
this one is not needed, if you check the whole dictionary the test above will fail already if there is a difference in the keys
| "grid_points", | ||
| "effective_area", | ||
| "energy_dispersion", | ||
| "psf_table", |
There was a problem hiding this comment.
instead of checking later on that psf_table is not in irf keys, you could simply remove it from here, same with background
2 participants
No description provided.