invert the logic from "skip" to "keep" - trying to improve readablility

[dneise]

@maxnoe maybe this improves readability .. maybe not .. was just a try

[dneise]

Ok so this broke a test related to allowed_tels .. which I do not understand .. my changes should not have an effect there ... interresting

[maxnoe]

Yes, it does, you did not invert the logic in the skip check, so now it get's the wrong number of events I think

[dneise]

I think I did invert it ... no? the new function check_keep says True when we want to keep the event.
And in line 171 where it is used, it says:

while not keep(event):
    read_next_event

[dneise]

Lunch time 👍

[dneise]

Ha, passed now ... I made a mistake before and replaced event.get('telescope_data') with "telescope_data" in event .. that was BS.

[maxnoe]

This might now return the telescope events instead of a bool. It might not be important, but could be strange.

[dneise]

oh!

[dneise]

I've added is_event_triggered = bool(event.get('telescope_events')) ... since I must admit in the beginning I did not immediately understand why we get the telescope events there. :D

I think that fixes the issue you found and might even improve readability... what do you think?

[dneise]

So @maxnoe do you think this conversion from "skip" to "keep" did improve anything with respect to readability? Or is it just in my brain and we should leave #214 as it is?

[maxnoe]

I would at least like to keep skip_integration, since that was already there before.

Then we have to ask if having two keyword arguments named skip_ with one having a double negative is better than one skip and one keep with no double negative.

I have the feeling that this will be very subjective.

[dneise]

Ah this is API changing? ... sorry I missed that.

Ok let's keep the skip_xxx and let's stick skip_xxx everywhere.