Feature step3 integration #45
Conversation
kmilo9999
commented
Dec 11, 2023
kmilo9999
commented
- Adding B03_plot_spectra_ov.py to the new packaging structure
|
@mochell when I run the command: Any thoughts ? |
There was a problem hiding this comment.
It think we'll need a new issue to remove the upper-case letter from this file's name, because they don't conform to PEP8. We could rename this file to startup.py or similar. But do this later and in one go once the package is cleaned up.
| def mean_spectral_error(self, mask=None, confidence=0.95): | ||
| return wavenumber_spectrogram.mean_spectral_error( |
There was a problem hiding this comment.
This method seems like it must be broken – wavenumber_spectrogram doesn't look to be defined anywhere. Do we use mean_spectral_error anywhere?
There was a problem hiding this comment.
There seems to be a wavenumber_spectrogram class in src/icesat2_tracks/ICEsat2_SI_tools/spectral_estimates.py with a mean_spectral_error method, imported as spec in L3.
| def mean_spectral_error(self, mask=None, confidence=0.95): | |
| return wavenumber_spectrogram.mean_spectral_error( | |
| def mean_spectral_error(self, mask=None, confidence=0.95): | |
| return spec.wavenumber_spectrogram.mean_spectral_error( |
That'd fix the linting problem but I'm not sure if it's used in some other file? Maybe the class doesn't need this method. @mochell
There was a problem hiding this comment.
ah, that'll be it - one of the imports handled by the old scripts which ran at the start of the file. Good catch, @cpaniaguam !
| if return_edges: | ||
| return Gmean, k_low_limits | ||
| else: | ||
| return Gmean |
There was a problem hiding this comment.
Maybe this is another thing worth checking with @mochell. The type of the returned objectn of this function is not stable and this requires special handling by the caller of the function. I think it's better to make it return a tuple (or better, a nameptuple) and make the downstream logic simpler. Maybe for a separate issue?
There was a problem hiding this comment.
I dont see rebin used in the files running the pipeline
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
What about returning the values Gmean,none . I can modify the only line in the whole project that needs this function to retrieve two values?
There was a problem hiding this comment.
If the caller of this function only needs one value, wouldn't it be better to make the function return Gmean only and make the required change?
| @@ -12,7 +12,7 @@ | |||
| import matplotlib | |||
| #matplotlib.use('Agg') | |||
There was a problem hiding this comment.
| #matplotlib.use('Agg') |
There was a problem hiding this comment.
###
# THIS FILE IS A LOCAL FILE
# it is not maintained via git, it contains configs specific to the machine
###
#os.environ["DISPLAY"] = "localhost:10.0"
# 14, 16, work
#standart libraries:
Has anyone given some thought to this? Should this be moved to configs?
| run: python src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py SH_20190502_05180312 SH_testSLsinglefile2 True | ||
| - name: third step make_spectra | ||
| run: python src/icesat2_tracks/analysis_db/B03_plot_spectra_ov.py SH_20190502_05180312 SH_testSLsinglefile2 True |
There was a problem hiding this comment.
| run: python src/icesat2_tracks/analysis_db/B03_plot_spectra_ov.py SH_20190502_05180312 SH_testSLsinglefile2 True | |
| run: python src/icesat2_tracks/analysis_db/B03_plot_spectra_ov.py SH_20190502_05180312 SH_testSLsinglefile2 True | |
Maybe formatters/pre-commit can be configured to add a new line at the end of files?
There was a problem hiding this comment.
let me add the formatter in another PR
There was a problem hiding this comment.
This might be a local setting, though.
| Gmean.k[Gmean.isel(x=slice(0, 20)).mean("x").argmax().data].data * 1, | ||
| Gmean.k[Gmean.isel(x=slice(0, 20)).mean("x").argmax().data].data * 1.25, | ||
| ) | ||
|
|
There was a problem hiding this comment.
The rest of the file deals with plotting. I think it's sensible to move these to another file/function to abstract away complexity. For a separate issue?
There was a problem hiding this comment.
yes. make a method out of it
chore: clean up startup.py
| def mean_spectral_error(self, mask=None, confidence=0.95): | ||
| return wavenumber_spectrogram.mean_spectral_error( |
There was a problem hiding this comment.
ah, that'll be it - one of the imports handled by the old scripts which ran at the start of the file. Good catch, @cpaniaguam !
| if return_edges: | ||
| return Gmean, k_low_limits | ||
| else: | ||
| return Gmean |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
chore: update class definitions
…2-tracks into feat-step3-intg
..
I think its just a bunch of warmings in cases when there is no data in the slice. should be solvable with an if statement.. |
