Laser test #84

nzywucka · 2023-11-29T21:00:16Z

No description provided.

…f container to keep the parameters.

…and to parse_laser_info().

jsitarek

I left a few comments.
In addition a test (following tests/...) should be added reading the information from some file, checked a few (in particular the tricky ones like the PheCounts) with their known values, and checking the interpretation of the MJD.

ctapipe_io_magic/__init__.py

…Container() class and parse_laser_info() function.

aleberti · 2023-12-04T10:50:15Z

Not sure why the tests are now failing. It complains about the fBadReport interpretation as bool, but if I do it on the files themselves at PIC, it does not complain (using the same version of uproot i.e. 5.1.2).

aleberti · 2023-12-04T10:58:50Z

Ok, I may have a hint. The files used for the tests are trimmed, i.e. they are not the full original files. So probably something went wrong there.

ctapipe_io_magic/__init__.py

codecov · 2023-12-04T15:34:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.38%. Comparing base (393d187) to head (3398c84).
Report is 15 commits behind head on master.

❗ Current head 3398c84 differs from pull request most recent head 399be3b. Consider uploading reports for the commit 399be3b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #84      +/-   ##
==========================================
+ Coverage   89.57%   90.38%   +0.81%     
==========================================
  Files          10        9       -1     
  Lines        1103     1207     +104     
==========================================
+ Hits          988     1091     +103     
- Misses        115      116       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

aleberti · 2023-12-04T15:36:09Z

Ok, I fixed the files. Now tests are passing. The Static Code analysis shows you have some blanck spaces in two lines, please remove them to make that pass as well.

ctapipe_io_magic/__init__.py

…ports. Filtering for the unique reports has been added.

…rected.

ctapipe_io_magic/__init__.py

Filtering the unique reports is improved.

Reading the unique reports has been improved

ctapipe_io_magic/__init__.py

ctapipe_io_magic/tests/test_lidar.py

ctapipe_io_magic/tests/test_magic_event_source.py

`# Please enter the commit message for your changes. Lines starting

Trailing whitespaces and unused libraries have been removed.

jsitarek

with the latest changes the PR looks good to me

aleberti

Just two small comments.

aleberti · 2024-04-24T09:23:46Z

ctapipe_io_magic/tests/test_lidar.py

+        assert report.IsUseGDAS == [False]
+        azimuth_degrees = report.Azimuth.value
+        assert azimuth_degrees == pytest.approx(276.63)
+        assert list(report.PheCounts) == [425, 764, 683, 711, 746, 721, 658, 731, 721, 677, 707, 754, 684, 728, 751, 711, 690, 695, 738, 687, 706, 650, 719, 724, 670, 710, 737, 742, 708, 705, 759, 179623, 2464563, 1758611, 1280821, 986393, 812893, 722703, 743947, 1691760, 3368201, 4376413, 4746189, 4724701, 4540566, 4299863, 4011618, 3763189, 3499434, 3253066, 3037985, 2831435, 2643358, 2474402, 2323471, 2178678, 2045924, 1925799, 1810810, 1706918, 1607277, 1515936, 1436227, 1356119, 1280398, 1218400, 1147896, 1086930, 1033397, 979359, 930597, 887483, 843525, 799898, 765021, 726615, 696215, 661314, 634164, 607068, 581747, 558592, 535797, 516721, 495480, 476815, 459176, 442030, 424442, 410585, 397295, 383475, 371108, 357603, 347352, 335689, 326221, 314597, 303917, 295578, 288181, 277774, 270177, 261659, 254427, 248027, 238576, 231383, 225801, 220153, 213184, 208052, 201773, 195992, 191629, 187563, 181116, 177353, 171939, 168135, 165829, 159561, 157485, 152367, 149774, 145431, 142500, 138534, 267828, 256294, 245388, 233980, 225530, 214241, 205567, 197794, 189473, 182911, 175534, 167783, 162659, 156169, 151238, 144940, 139989, 136526, 130620, 126002, 122111, 118354, 114672, 111281, 106334, 103802, 99964, 97115, 93948, 91785, 88386, 86373, 84068, 81654, 78832, 77558, 75214, 72567, 70234, 68192, 66504, 65121, 62752, 61142, 60156, 57868, 56230, 54986, 53344, 53607, 52631, 49575, 49334, 47419, 46824, 45539, 43945, 42663, 42277, 42386, 40259, 39347, 38583, 38002, 37322, 36039, 35217, 34848, 34428, 33457, 32524, 31969, 31401, 30554, 30320, 29564, 28803, 28435, 27901, 27275, 26462, 26659, 26215, 25136, 24792, 24609, 23965, 23048, 23350, 23042, 22148, 21619, 21450, 21027, 21040, 20045, 19757, 19730, 19343, 19072, 18729, 19274, 17827, 17871, 17421, 17081, 16846, 16688, 16752, 16059, 15695, 15649, 15363, 15133, 15618, 15505, 15868, 18740, 20848, 19722, 18449, 15898, 14839, 13871, 13201, 13033, 13031, 12772, 23447, 22891, 21864, 21490, 20727, 20305, 19659, 18968, 18497, 18345, 17716, 17234, 16854, 16291, 15821, 15401, 15487, 15011, 14650, 14303, 13954, 13555, 13169, 13125, 12681, 12523, 12188, 11897, 11597, 11074, 11024, 10835, 10722, 10498, 10181, 10086, 9662, 9426, 9273, 9142, 8951, 8862, 8628, 8530, 8224, 8264, 8024, 7831, 7822, 7726, 7589, 7503, 7280, 7255, 7056, 6966, 6956, 6777, 6780, 6572, 6482, 6485, 6429, 6262, 6201, 6088, 6046, 5915, 5924, 5769, 5687, 5625, 5524, 5506, 5426, 5379, 5379, 5212, 5215, 5132, 5266, 5106, 5101, 5003, 4974, 4811, 4942, 4740, 4822, 4714, 4626, 4698, 4632, 4568, 4574, 4336, 4467, 4400, 4299, 4255, 4295, 4255, 4122, 4339, 4209, 4181, 4100, 4080, 4115, 4121, 4102, 4002, 3896, 3971, 3894, 3909, 3898, 3835, 3782, 3776, 3699, 3779, 3660, 3696, 3650, 3667, 3685, 3681, 7131, 7254, 7312, 7134, 6978, 7045, 6834, 6834, 6788, 6687, 6639, 6634, 6511, 6787, 6603, 6464, 6388, 6437, 6419, 6323, 6360, 6418, 6294, 6370, 6247, 6239, 6241, 6178, 6056, 6059, 5981, 6160, 6118, 5884, 5885, 5893, 5958, 5988, 5869, 6088, 5947, 5961, 5964, 5878, 5866, 5879, 5751, 5918, 5819, 5843, 5750, 5869, 5920, 5854, 5863, 5797, 5856, 5688, 5788, 5754, 5690, 5799, 5664, 5863, 5601, 5660, 5693, 5790, 5886, 5787, 5792, 5792, 5665, 5651, 5771, 5831, 5640, 5804, 5748, 5696, 5758, 5704, 5567, 5712, 5693, 5839, 5748, 5725, 5830, 5675, 5586, 5837, 5627, 5727, 5710, 5603, 5709, 5637, 5752, 5886, 5757, 5639, 5726, 5677, 5650, 5696, 5654, 5832, 5692, 5530, 5694, 5706, 5675, 5608, 5683, 5595, 5657, 5745, 5596, 5759, 5737, 5731, 5630, 5671, 5683, 5632, 5695, 5705]


I would define the long list above, giving it a meaningful name e.g. phe_counts, so that this line becomes:

list(report.PheCounts) == phe_counts

I resolved the conversation, but actually I see no new commit. Maybe you forgot to push your changes?

ctapipe_io_magic/__init__.py

nzywucka added 3 commits November 23, 2023 15:06

Reading of uncalibrated reports from Magic lidar and implementation o…

3bc1bf2

…f container to keep the parameters.

Removed print.

fbd80b1

In __init__.py, more parameters were added to ReportLaserContainer() …

7a97565

…and to parse_laser_info().

jsitarek requested changes Nov 30, 2023

View reviewed changes

jsitarek reviewed Nov 30, 2023

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Nov 30, 2023

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

__init__.py changed regarding Laser test #84 comments in ReportLaser…

3398c84

…Container() class and parse_laser_info() function.

aleberti requested changes Dec 4, 2023

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Dec 5, 2023

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

nzywucka added 7 commits February 26, 2024 12:17

Unnecessary spaces are removed from the file.

867aa73

Unnecessary spaces are removed

aad168a

Unnecessary spaces removed

9cebd79

Typo removed

74d5c62

If statement for self.laser = self.parse_laser_info() added

8bc6487

Typo removed

e0c1118

BGSamples parameter added

7e0252f

aleberti requested changes Feb 26, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

nzywucka added 4 commits March 12, 2024 09:19

The [0]s have been removed from the arrays while reading the Lidar re…

be6a536

…ports. Filtering for the unique reports has been added.

Unindent that does not match any outer indentation level has been cor…

6c424c8

…rected.

Filtering for the unique reports has been corrected.

f5e1669

Unused variable has been removed.

27d8a8f

jsitarek reviewed Mar 12, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 12, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 12, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

nzywucka added 2 commits March 12, 2024 11:28

Reading of the unnecesary parameters from the lidar reports are removed.

4135a5e

Filtering the unique reports is improved.

Unused import has been removed

7d6e7ce

Reading the unique reports has been improved

nzywucka added 5 commits March 27, 2024 11:06

test_lidar_parameters was removed

1864e76

Implementation of some tests of reading lidar parameters

61f4a75

test_lidar_parameters test has been improved.

94efd5e

Unnecessary comments and whitespaces have been removed.

d46221f

Reading BadReport and State parameters added.

fadf6d4

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/__init__.py Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/tests/test_lidar.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/tests/test_lidar.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/tests/test_lidar.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/tests/test_lidar.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/tests/test_lidar.py Outdated Show resolved Hide resolved

jsitarek reviewed Mar 28, 2024

View reviewed changes

ctapipe_io_magic/tests/test_magic_event_source.py Outdated Show resolved Hide resolved

nzywucka added 5 commits April 2, 2024 18:56

Reading unique reports changed.

fce7374

Remove assert from tests in codacy

25136f0

`# Please enter the commit message for your changes. Lines starting

Skipping assert checks in the test files.

9f14bb5

Trailing whitespaces and unused libraries have been removed.

Skipping assert checks in the test files.

20091c7

Tests for reading lidar parameters have been removed.

4f8597c

jsitarek approved these changes Apr 3, 2024

View reviewed changes

aleberti requested changes Apr 24, 2024

View reviewed changes

MJD loop is put outside the try .. except constuction

399be3b

aleberti approved these changes Apr 25, 2024

View reviewed changes

aleberti merged commit 0d7153d into master Apr 25, 2024
3 of 4 checks passed

aleberti deleted the laser_test branch April 25, 2024 11:17

nzywucka restored the laser_test branch October 30, 2024 09:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Laser test #84

Laser test #84

nzywucka commented Nov 29, 2023

jsitarek left a comment

aleberti commented Dec 4, 2023

aleberti commented Dec 4, 2023

codecov bot commented Dec 4, 2023 •

edited

Loading

aleberti commented Dec 4, 2023 •

edited

Loading

jsitarek left a comment

aleberti left a comment

aleberti Apr 24, 2024

nzywucka Apr 24, 2024

aleberti Apr 24, 2024

Laser test #84

Laser test #84

Conversation

nzywucka commented Nov 29, 2023

jsitarek left a comment

Choose a reason for hiding this comment

aleberti commented Dec 4, 2023

aleberti commented Dec 4, 2023

codecov bot commented Dec 4, 2023 • edited Loading

Codecov Report

aleberti commented Dec 4, 2023 • edited Loading

jsitarek left a comment

Choose a reason for hiding this comment

aleberti left a comment

Choose a reason for hiding this comment

aleberti Apr 24, 2024

Choose a reason for hiding this comment

nzywucka Apr 24, 2024

Choose a reason for hiding this comment

aleberti Apr 24, 2024

Choose a reason for hiding this comment

codecov bot commented Dec 4, 2023 •

edited

Loading

aleberti commented Dec 4, 2023 •

edited

Loading