Conversation
imaliyov
left a comment
imaliyov
left a comment
There was a problem hiding this comment.
Thank you, Yao, this is a lot better!
Next time, please do not close the initial pull request, but continue commiting there. In this case, it will be easier to track changes.
Unless you had to change a lot...
Please add just one test that I specified in the file and we'll be good to go!
hurricane642
left a comment
hurricane642
left a comment
There was a problem hiding this comment.
Overall looks good, thank you!
My comments mainly on the documentation of the functions
| temp_array : numpy.ndarray | ||
| Array of temperatures corresponding to the spectral data. | ||
| freq_array : numpy.ndarray | ||
| Array of energy values (ω) in electron volts (eV). |
There was a problem hiding this comment.
I'm not sure about usage of ω in the code - could be issues with this kind of symbols
|
|
||
| Parameters | ||
| ---------- | ||
| plt_loc : matplotlib.pyplot |
There was a problem hiding this comment.
Why do we need to pass the module to the function?
|
|
||
| Parameters | ||
| ---------- | ||
| popu_path : str |
There was a problem hiding this comment.
Update documentation on this function
Co-authored-by: Sergei Kliavinek <klyavinekss@gmail.com>
Co-authored-by: Sergei Kliavinek <klyavinekss@gmail.com>
| temp_array : numpy.ndarray | ||
| Array of temperatures corresponding to the spectral data. | ||
| freq_array : numpy.ndarray | ||
| Array of energy values (ω) in electron volts (eV). |
I click the resolve when I changed the code. Now it is updated. |
hurricane642
left a comment
hurricane642
left a comment
There was a problem hiding this comment.
Cool, LGTM! Thanks!
3 participants
All the comments from last PR are fixed. Thank you, Ivan!