Adjust import statements to match convention #43
Conversation
There was a problem hiding this comment.
Good job overall! I only marked two spots where you missed the np. prefix, both times before the 'sum' function. We gotta be careful with this specifically, since there is a function called 'sum' in standard python, so removing the import for 'sum' from numpy does not lead to a syntax error.
napytau/core/tau_final.py
Outdated
| @@ -24,12 +21,12 @@ def calculate_tau_final( | |||
| if len(tau_i_values) == 0: | |||
| return -1, -1 | |||
|
|
|||
| weights: ndarray = 1 / power(delta_tau_i_values, 2) | |||
| weights: np.ndarray = 1 / np.power(delta_tau_i_values, 2) | |||
|
|
|||
| # Calculate the weighted mean of tau_i | |||
| weighted_mean: float = sum(weights * tau_i_values) / sum(weights) | |||
There was a problem hiding this comment.
| weighted_mean: float = sum(weights * tau_i_values) / sum(weights) | |
| weighted_mean: float = np.sum(weights * tau_i_values) / np.sum(weights) |
There was a problem hiding this comment.
Its concerning, that the tests didn't catch that, could you have a look at why this wasn't tested?
There was a problem hiding this comment.
Yes, that's what I thought as well. Turns out I forgot to verify the mocked functions in tau_final are called correctly. I now added the tests for this.
Co-authored-by: Martin Fuchs <[email protected]>
napytau/core/chi.py
Outdated
| from numpy import sum | ||
| from numpy import mean | ||
| from numpy import power | ||
| import numpy as np | ||
| from scipy import optimize | ||
| from scipy.optimize import OptimizeResult |
There was a problem hiding this comment.
this does not match the import convention
Summary
Adjusted all numpy import statements to match and create continuous scheme.
PR checklist
Closes/Fixes/Related to