gnn_tracking
gnn_tracking copied to clipboard
Create tests for `utils/plotting.py`
Hey @klieret Can I work on this?
Sure. Probably some things can be tested rather trivially with the test data in the repository.
@klieret Do you think these files should be renamed (or fix the code)?
Especially because of this line: https://github.com/gnn-tracking/gnn_tracking/blob/75cc6020e5db33064d82b2330d15783aa72cefc2/src/gnn_tracking/utils/plotting.py#L123
- This already assumes that the prefix is
event
(but here it is test_event). - In the example notebook, we use this
event{evtid:09}-cells.csv.gz
to set event_id to be a 9-letter string. But in this line (event0000{evtid}
), the prefix won't be a 9-lettered string ifevtid
is less than 10000.
I can work with test data by renaming them, but by any chance does this needs to be fixed?
Hi! Thanks, you're absolutely right; that's some bad stuff right there.
I'd say we do the following:
- Renaming the test files to start with
event
(like the original data) - Changing
event0000{evtid}
also toevent{evtid:09}
Thank you for taking such close a look!
I am wondering whether there is a need for these two functions plot_3d
, plot_rz
(not part of any class; independent functions) as I don't see them used anywhere in the codebase.
A lot of the plotting functions are used for analysis, which is done in separate notebooks. However, these two functions are part of the legacy codebase and they look somewhat less fleshed out than the rest.
I'd say: Let's keep them, but no need to test them for now :)