replacing median with 50th percentile #97
Conversation
Codecov Report
@@ Coverage Diff @@
## main #97 +/- ##
==========================================
+ Coverage 74.39% 74.44% +0.05%
==========================================
Files 32 32
Lines 2882 2888 +6
==========================================
+ Hits 2144 2150 +6
Misses 738 738
|
There was a problem hiding this comment.
Nice catch, thanks! Your proposed solution looks good to me!
Regarding code cleanliness, I think it would make sense to create a helper method for the 50th percentile, so that the keyword args don't have to be repeatet.
def fiftieth_percentile(data):
return np.percentile(data, 50, interpolation='nearest')And please make sure to add a test ;)
|
@oliversus do you have time to fix this soon? |
Hi Martin and Stephan, |
| @@ -820,7 +820,26 @@ def get_angles(self): | |||
| arr[self.mask] = np.nan | |||
|
|
|||
| return sat_azi, sat_zenith, sun_azi, sun_zenith, rel_azi | |||
|
|
|||
| def _get_true_median(self, input_array, q=50, interpolation='nearest'): | |||
There was a problem hiding this comment.
Oh wait :D If you have some spare time left...
This method doesn't access any class attribute, so I'd propose to move it out of the class to the module level. Or maybe to pygac.utils?
Method
correct_times_medianshould provide median values for year, doy, and milliseconds to correct invalid timestamps. However,np.medianwill return floats, which could even be fractions due tonp.median's internal interpolation method of the two closest values. I'd suggest to usenp.percentileinstead, where one can explicitly set the interpolation method. Setting this to "nearest" will always return the value closest to the statistical median, thus not changing its type.np.median([1, 2, 3, 4, 5, 6])returns 3.5
np.percentile([1, 2, 3, 4, 5, 6], 50, interpolation="nearest")returns 3
This will also avoid a
ValueErrorin methodto_datetime64, line 449, where conversion to datetime64 is only possible with year as an integer.