Integrate step 2 #44

kmilo9999 · 2023-12-06T21:04:11Z

Modified m_spectrum_ph3.py and ICEsat2_SI_tools/generalized_FT.py to integrate the pipeline's step 2 to the current folder structure and packaging configuration

cpaniaguam

I think this is good! We can do some cleanups later.

hollandjg

There seems to be an install problem here!

.github/workflows/test-B01_SL_load_single_file.yml

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

hollandjg · 2023-12-11T15:55:25Z

pyproject.toml

I don't seem to be able to install this using the description in the README any more – I'm getting the following error:

Getting requirements to build wheel ... error error: subprocess-exited-with-error × Getting requirements to build wheel did not run successfully. │ exit code: 1 ╰─> [12 lines of output] /var/folders/n5/6b48sz2j3yldl4mnglvsr6mh0000gq/T/H5closem9scgpzm.c:1:10: fatal error: 'H5public.h' file not found #include "H5public.h" ^~~~~~~~~~~~ 1 error generated. cpuinfo failed, assuming no CPU features: 'flags' * Using Python 3.9.18 (main, Oct 4 2023, 15:25:11) * Found cython 3.0.6 * USE_PKGCONFIG: True .. ERROR:: Could not find a local HDF5 installation. You may need to explicitly state where your local HDF5 headers and library can be found by setting the ``HDF5_DIR`` environment variable or by using the ``--hdf5`` command-line option. [end of output] note: This error originates from a subprocess, and is likely not a problem with pip. error: subprocess-exited-with-error × Getting requirements to build wheel did not run successfully. │ exit code: 1 ╰─> See above for output. note: This error originates from a subprocess, and is likely not a problem with pip.

Is there something special I need to do to make this work? On main it still builds fine. It looks like a problem with tables.

@kmilo9999 check when tables is being used.

@hollandjg This error is because it's not finding HDF5 in your macos. I created a workflow that checks the installation in that system and they seem to work. Have you set the HDF5_DIR environmental variable?

@cpaniaguam @hollandjg This is the stack trace of the throw error when the pytables dependency is missed.

Traceback (most recent call last): File "/mnt/c/Projects/Github/icesat2-tracks/src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py", line 439, in <module> MT.save_pandas_table(Pars_optm, save_name+'_params', save_path ) File "/mnt/c/Projects/Github/icesat2-tracks/src/icesat2_tracks/local_modules/m_tools_ph3.py", line 282, in save_pandas_table with HDFStore(save_path+'/'+ID+'.h5') as store: File "/mnt/c/Projects/Github/icesat2-tracks/venv39/lib/python3.9/site-packages/pandas/io/pytables.py", line 566, in __init__ tables = import_optional_dependency("tables") File "/mnt/c/Projects/Github/icesat2-tracks/venv39/lib/python3.9/site-packages/pandas/compat/_optional.py", line 135, in import_optional_dependency raise ImportError(msg) ImportError: Missing optional dependency 'pytables'. Use pip or conda to install pytables.

Basically, when the analysis results are being save to a pandas table, pandas calls the function :
import_optional_dependency("tables")
and that producess the error

This solves it, but I think it needs to be in the README. I've put in a suggestion in #49 which can be merged into this branch.

add some instructions for HDF5-dependency #49

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

… to work with it

cpaniaguam

Suggested a couple more cleanups before approving and merging.

Also added some comments for future development items.

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

cpaniaguam · 2023-12-14T17:28:15Z

pyproject.toml

+  "sliderule==4.1.0",
+  "ipyleaflet",
+  "lmfit",
+  "tables"


We might be able to remove this dep if the I/O is optimized in step 2.

That would be nice.

cpaniaguam · 2023-12-14T17:30:35Z

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

@@ -50,7 +58,7 @@ def linear_gap_fill(F, key_lead, key_int):
    return y_g


-# %%
+
 track_name, batch_key, test_flag = io.init_from_input(sys.argv) # loads standard experiment


This might change after the cli is implemented.

Absolutely!

Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

cpaniaguam

Pointed out a few more lines that can also be deleted.

Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

…T.py

cpaniaguam

I am requesting one more deletion of commented code. Other than that I think this is ready to merge!

cpaniaguam · 2023-12-15T15:27:31Z

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

 # h.heap()
 #import s3fs
 #from memory_profiler import profile


Why not delete these too?

cpaniaguam · 2023-12-15T15:31:11Z

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

-#track_name, batch_key, test_flag = '20190601094826_09790312_004_01', 'SH_batch01', False
-#track_name, batch_key, test_flag = '20190207111114_06260210_004_01', 'SH_batch02', False
-#track_name, batch_key, test_flag = '20190208152826_06440210_004_01', 'SH_batch01', False
-#track_name, batch_key, test_flag = '20190213133330_07190212_004_01', 'SH_batch02', False
 #track_name, batch_key, test_flag = '20190205231558_06030212_004_01', 'SH_batch02', False


I might have missed suggesting removing other commented code, but lines 61-76, 78, 106 could be deleted.

I removed all the commented code

cpaniaguam

Thanks!

cpaniaguam · 2023-12-15T17:02:56Z

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

Clean as a whistle!

hollandjg

Love it! Just one suggested change in #49

kmilo9999 added 11 commits December 5, 2023 20:58

finished step2 integration

f0ef135

Merge branch 'main' into feat-step2-intg

6c852f5

added missed file B02_make_spectra_gFT. modifed workflow to test step2

e71e1c7

add lmfit module to pyproject.toml

8820d2d

removing sttep 2 from test workflow

e27ee64

removing dependency to test workkflow

3dac298

adding lmfit again to test step 1

28b253d

adding step 2 in workflow

ce05d64

add pytables as dependency

0f706f0

remove pytables as dependency

701d18d

added tables depedency

ae9fba6

kmilo9999 requested review from cpaniaguam and hollandjg December 7, 2023 15:47

removing B02_make_spectra_gFT.py file from root folder

0a4edaa

cpaniaguam previously approved these changes Dec 11, 2023

View reviewed changes

hollandjg requested changes Dec 11, 2023

View reviewed changes

renaming step workflow and removing unnecessary code

b02a4b2

kmilo9999 dismissed cpaniaguam’s stale review via b02a4b2 December 12, 2023 22:01

new sliderule version came out a few weeks ago and the pipeline seems…

e3ebaf8

… to work with it

kmilo9999 requested review from hollandjg and cpaniaguam December 13, 2023 20:23

cpaniaguam reviewed Dec 14, 2023

View reviewed changes

cpaniaguam self-requested a review December 14, 2023 17:32

Apply suggestions from code review

7e93e0b

Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

cpaniaguam reviewed Dec 14, 2023

View reviewed changes

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py Outdated Show resolved Hide resolved

cpaniaguam requested changes Dec 14, 2023

View reviewed changes

kmilo9999 and others added 2 commits December 15, 2023 09:46

Apply suggestions from code review

8d210c7

Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

removed all commented code and formating the file B02_make_spectra_gF…

67846f0

…T.py

kmilo9999 requested a review from cpaniaguam December 15, 2023 14:46

cpaniaguam requested changes Dec 15, 2023

View reviewed changes

kmilo9999 requested a review from cpaniaguam December 15, 2023 15:47

cpaniaguam approved these changes Dec 15, 2023

View reviewed changes

src/icesat2_tracks/analysis_db/B02_make_spectra_gFT.py

Copy link

Member

cpaniaguam Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean as a whistle!

hollandjg approved these changes Dec 15, 2023

View reviewed changes

kmilo9999 merged commit a497b80 into main Dec 15, 2023
1 check passed

kmilo9999 deleted the feat-step2-intg branch December 15, 2023 21:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate step 2 #44

Integrate step 2 #44

kmilo9999 commented Dec 6, 2023

cpaniaguam left a comment

hollandjg left a comment

hollandjg Dec 11, 2023

kmilo9999 Dec 12, 2023

kmilo9999 Dec 13, 2023

hollandjg Dec 15, 2023

cpaniaguam left a comment

cpaniaguam Dec 14, 2023

hollandjg Dec 15, 2023

cpaniaguam Dec 14, 2023

hollandjg Dec 15, 2023

cpaniaguam left a comment

cpaniaguam left a comment

cpaniaguam Dec 15, 2023

cpaniaguam Dec 15, 2023

kmilo9999 Dec 15, 2023

cpaniaguam left a comment

cpaniaguam Dec 15, 2023

hollandjg left a comment

Integrate step 2 #44

Integrate step 2 #44

Conversation

kmilo9999 commented Dec 6, 2023

cpaniaguam left a comment

Choose a reason for hiding this comment

hollandjg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpaniaguam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpaniaguam left a comment

Choose a reason for hiding this comment

cpaniaguam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpaniaguam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hollandjg left a comment

Choose a reason for hiding this comment