Refactor NuRadioReco/modules/efieldToVoltageConverter.py #707
Conversation
|
ping |
|
Is it clear to you why the unit tests are failing? It's not clear to me, but they don't seem to fail on other branches as far as I can tell... |
|
Hm I am puzzled. I did not change anything? |
|
A miracle, the tests run through now ... |
|
@sjoerd-bouma could you do me the honour of approving? |
|
ping |
| self.__uncertainty['sys_dz'] = np.random.normal(0, self.__uncertainty['sys_dz']) | ||
| if('sys_amp' in self.__uncertainty): | ||
| for iCh in self.__uncertainty['sys_amp'].keys(): | ||
| self.__uncertainty = uncertainty or {} |
There was a problem hiding this comment.
I wonder what percentage of python users finds this more intuitive than the four-line equivalent... I'm not a convert yet.
There was a problem hiding this comment.
if that is the case: ciao.
| @@ -267,3 +266,39 @@ def end(self): | |||
| dt = timedelta(seconds=self.__t) | |||
| self.logger.info("total time used by this module is {}".format(dt)) | |||
| return dt | |||
|
|
|||
|
|
|||
| def calculate_time_shift_for_cosmic_ray(det, station, efield, channel_id): | |||
There was a problem hiding this comment.
Also not sure if it's worth making this function private, I guess we don´t expect it to be used on its own. But this is anyway not something we're very consistent about throughout NuRadio.
There was a problem hiding this comment.
next PR? Is this function useful for the other efield2Voltage converter modules?
There was a problem hiding this comment.
Yeah I'm happy for this to be merged as is.
Moving some repetitive code in its own function.