add parameters to delete downloaded data from spectroscopy queries #347

jkrick · 2024-09-25T21:22:13Z

added delete downloaded data to JWST, HST and made uniform variable names with existing Herschel function. The other archives don't download any data.
cleaned up the order of the cells (JWST had gotten separated from the MAST section)
removed intro section on 'open issues' and moved these to actual issues. Its easier not to remember we need to edit this section every time we make changes.
updated runtime info for the notebook. It now takes 30s longer than before, which is ~10%. Not sure why, and I don't have the statistics to know if this is only this notebook (which hits many archives some of which could be having a slow day), or if it is the newer system.

This will close #285

bsipocz

I wonder if the default for the delete_downloaded_data should be False, but we can iterate on that.

Some minor code cleanup suggestions, otherwise the things I notices should come up in #336, e.g. making more keywords optional, pep8 cleanup.

bsipocz · 2024-09-27T21:29:37Z

spectroscopy/code_src/herschel_functions.py

@@ -39,7 +39,7 @@ def find_max_flux_column(df):
  return max_flux_col


-def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_tarfiles = False):
+def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_downloaded_data = True):


Suggested change

def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_downloaded_data = True):

def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_downloaded_data=True):

spectroscopy/code_src/herschel_functions.py

spectroscopy/code_src/mast_functions.py

bsipocz · 2024-09-27T21:34:58Z

spectroscopy/spectra_generator.md

-```{code-cell}
+```{code-cell} ipython3
 # Uncomment the next line to install dependencies if needed.
-# !pip install -r requirements_spectra_generator.txt
+!pip install -r requirements_spectra_generator.txt


Please don't commit changes to this cell.

Its hard for me to remember this because I need it uncommented to run, and then forget to go back and comment it out. Is there some easy way to have both work, ie some if statement around it, or???

Yes, I don't have a good solution for how to make it more convenient to commit patches rather than full files when working directly on fornax.

However, I'm surprised that you need to rerun the installs each time, my understanding was that you need to install the extra ones only once and they will be in your persistent storage space.

Every time I login to Fornax, I need to re-run the pip install cell. Installs are persistent across kernel restarts but not daskhub logouts.

Ahh, interesting. I'll open a follow-up issue for this over at the image repo as I think it would be a nicer user experience to have persistency. (But also, installing all these requirements file is on the request list)

ie some if statement around it, or

While we did some conditionals and then install steps in the early version, I don't think it's particularly good to ship demo notebooks to end users where installations are hidden away that way. cc @troyraen

As a workflow workaround for you as a notebook developer rather than end user could be to pip install everything from a terminal session freshly after a new login and then propagate the list of missing dependencies as an issue in the fornax-image repo, so we can include it in the kernel.

spectroscopy/spectra_generator.md

jkrick · 2024-09-30T02:05:01Z

I was thinking the default needs to be True to delete downloaded data because of the initial space limit on Fornax.

jkrick

Thanks for looking at it, all changes made.

jkrick · 2024-09-30T15:04:07Z

spectroscopy/code_src/herschel_functions.py

@@ -39,7 +39,7 @@ def find_max_flux_column(df):
  return max_flux_col


-def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_tarfiles = False):
+def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_downloaded_data = True):


spectroscopy/code_src/herschel_functions.py

spectroscopy/code_src/mast_functions.py

jkrick · 2024-09-30T15:08:57Z

spectroscopy/spectra_generator.md

-```{code-cell}
+```{code-cell} ipython3
 # Uncomment the next line to install dependencies if needed.
-# !pip install -r requirements_spectra_generator.txt
+!pip install -r requirements_spectra_generator.txt


Its hard for me to remember this because I need it uncommented to run, and then forget to go back and comment it out. Is there some easy way to have both work, ie some if statement around it, or???

spectroscopy/spectra_generator.md

bsipocz · 2024-09-30T18:08:35Z

I was thinking the default needs to be True to delete downloaded data because of the initial space limit on Fornax.

That would mean a lot of duplicated download when the users are experimenting in the same notebook.
I would just hope some bigger temporary workspace will be available, a space that doesn't have backup or anything fancy, but is suitable for dumping content during a work session.
cc @zoghbi-a - Is that on the radar or do we have to cook up workarounds to operate in a tiny box?

bsipocz · 2024-09-30T18:10:32Z

@jkrick - are the any more commits to be pushed here? You said for some of the review comments that they have been done, though they have not (if that's easier, I have left them as review comments, so all of those can be committed directly without editing the file manually)

jkrick · 2024-09-30T20:34:52Z

Sorry @bsipocz I thought I got them all. Now it is telling me 'outdated suggestions cannot be applied'. I did the changes manually to try to help learn from the suggestions, but I must have missed something.

bsipocz

OK, so here is another batch. I'll commit these and will go ahead with the merge, and keep the True or False is better for default for #336, or to be totally separated pending we get some roadmap for a better temporary storage situation.

spectroscopy/code_src/herschel_functions.py

spectroscopy/code_src/mast_functions.py

spectroscopy/spectra_generator.md

add parameters to delete downloaded data from spectroscopy queries df06e2f

jkrick added 6 commits September 25, 2024 16:19

cleaning up order of cells

df89f06

added delete_data to HST function

72d0ca3

renaming herschel delete_tarfiles to delete_downloaded_data

139165d

fixing HST delete_downloaded_data to work

244d6f1

added delete_downloaded_data to JWST functions

7cf0e2b

removed open issues section from intro

e379463

jkrick added the use case: spectroscopy Spectroscopy use case label Sep 25, 2024

jkrick self-assigned this Sep 25, 2024

jkrick requested a review from bsipocz September 25, 2024 21:23

jkrick mentioned this pull request Sep 25, 2024

make cleanup parameter for storing/deleting downloaded files for _get_spec functions #285

Closed

bsipocz approved these changes Sep 28, 2024

View reviewed changes

edits from review including line spacing and import, pip installs, etc.

1893553

jkrick commented Sep 30, 2024

View reviewed changes

bsipocz mentioned this pull request Sep 30, 2024

EHN: user installs to be done in a persistent directory nasa-fornax/fornax-images#16

Open

bsipocz approved these changes Sep 30, 2024

View reviewed changes

Minor cleanups before merging [skip ci]

cf75254

bsipocz merged commit df06e2f into main Sep 30, 2024

bsipocz deleted the delete_data_files branch September 30, 2024 20:49

github-actions bot pushed a commit that referenced this pull request Sep 30, 2024

Merge pull request #347 from nasa-fornax/delete_data_files

490b3c7

add parameters to delete downloaded data from spectroscopy queries df06e2f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add parameters to delete downloaded data from spectroscopy queries #347

add parameters to delete downloaded data from spectroscopy queries #347

jkrick commented Sep 25, 2024

bsipocz left a comment

bsipocz Sep 27, 2024

jkrick Sep 30, 2024

bsipocz Sep 27, 2024

jkrick Sep 30, 2024

bsipocz Sep 30, 2024

jkrick Sep 30, 2024

bsipocz Sep 30, 2024

bsipocz Sep 30, 2024

jkrick commented Sep 30, 2024

jkrick left a comment

jkrick Sep 30, 2024

jkrick Sep 30, 2024

bsipocz commented Sep 30, 2024

bsipocz commented Sep 30, 2024

jkrick commented Sep 30, 2024

bsipocz left a comment

	def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_downloaded_data = True):
	def Herschel_get_spec(sample_table, search_radius_arcsec, datadir, delete_downloaded_data=True):

add parameters to delete downloaded data from spectroscopy queries #347

add parameters to delete downloaded data from spectroscopy queries #347

Conversation

jkrick commented Sep 25, 2024

bsipocz 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

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

jkrick commented Sep 30, 2024

jkrick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsipocz commented Sep 30, 2024

bsipocz commented Sep 30, 2024

jkrick commented Sep 30, 2024

bsipocz left a comment

Choose a reason for hiding this comment