added acs_focus_diverse_epsfs for review

[gsanand]

acstools 3.7.0 is now available, so the notebook is ready for review

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[haticekaratay]

acstools 3.7.0 is now available, so the notebook is ready for review

Thank you, @gsanand, for submitting your notebook. We have added it to our queue for technical review.

[haticekaratay]

To begin the review, please address execution errors from the logs and style errors shown here.

[gsanand]

hi @haticekaratay I fixed the PEP8 errors but I can't figure out how to deal with the execution issue. it is supposed to download a file, so maybe it is looking for it in some place that is not obvious to me?

[haticekaratay]

Hi @gsanand, running this notebook locally works well for file location as you mentioned, but it seems the CI environment is having trouble locating the file. To improve this, I have a couple of suggestions. First, consider specifying a specific download directory instead of using './'. This can help ensure consistent file paths in the CI environment, where the directory structure often differs from local setups. Second, using os.path.abspath to convert the relative path to an absolute one could clarify the file's location, aiding the CI system in locating the file more reliably. Additionally, I recommend using a context manager when opening files with fits.open. 

import os # add this with other imports
# Set the download location to the current working directory
download_location = os.path.join(os.getcwd(), 'downloads')
os.makedirs(download_location, exist_ok=True)
retrieved_download = psf_retriever('jds408jsq', download_location) 
retrieved_filepath = os.path.abspath(retrieved_download)
if not os.path.isfile(retrieved_filepath):
  raise FileNotFoundError(f"Expected file not found at {retrieved_filepath}")
with fits.open(retrieved_filepath) as hdu:
  hdu.info() # Display basic information about the file

Reply via ReviewNB

[gsanand]

hi @haticekaratay, I tried your suggestions above, and while it still works locally, it does not get rid of the CI errors on git. any thoughts as to what is going on? the errors are sufficiently vague enough that I'm not entirely sure what the issue is

[mgough-970]

The notebook was failing execution in CI - I narrowed it down to an acstools function, while creating the issue report for acstools I attempted to run it today and suddenly .... the notebook executes.

There must have been some downstream issue that was resolved, I'm not sure what that is, but the problem is gone.

One additional note, this PR is from a fork to the repo main, we cannot do this, it will not properly push the outputs to storage branch and will not generate HTML, please merge to a branch here first, then PR to main. Thanks!

[haticekaratay]

Thanks for the update, @mgough-970; I will handle the branching issue.

[haticekaratay]

Hello @dulude and @mgough-970,

We're facing an issue with our CI where it is unable to locate the retrieved_download. We need your assistance in figuring out the root cause of the problem.

We have tested the notebook locally and it works perfectly fine. We even tried adding the downloads directory to ensure that CI would find the correct path, but unfortunately, it still can't find the downloads.

We would greatly appreciate any help you can offer us. Thank you.

[dulude]

We're facing an issue with our CI where it is unable to locate the retrieved_download. We need your assistance in figuring out the root cause of the problem.

We have tested the notebook locally and it works perfectly fine. We even tried adding the downloads directory to ensure that CI would find the correct path, but unfortunately, it still can't find the downloads.

We would greatly appreciate any help you can offer us. Thank you.

Hey @gsanand , a possible stop-gap solution to this might be to skip use of psf_retriever all together for now and download the data with astroquery, as you do further down in the notebook. Astroquery widely used in numerous other notebooks without issue. That way, you could get the notebook published now and we can circle back to the issue later.

[gsanand]

Hi @dulude, if I understand your suggestion correctly, then that is not possible, as the ePSFs are only available through the custom AWS API that I have set up (and not through MAST or anything like that).

[haticekaratay]

#195 is created to enable notebook deployment on the GitHub pages.