Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrected HTML Link #340

Merged
merged 2 commits into from
Dec 23, 2024
Merged

Conversation

mrevalski
Copy link
Collaborator

The link to the revised WFPC2 drizzling notebook in the README.md file has been corrected from *.ipynb to an *.html extension.

@mrevalski
Copy link
Collaborator Author

Hi @gibsongreen, this request corrects a minor error in the URL for the revised WFPC2 drizzling notebook that was merged in PR #333. However, I receive a 404 error when trying to navigate to the HTML page. I do not see my new branch listed under the ci_manual_single_merge_generate workflow request, so it may need to be generated via another manual workflow? Thank you!

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@gibsongreen
Copy link
Collaborator

gibsongreen commented Dec 20, 2024

Hi @gibsongreen, this request corrects a minor error in the URL for the revised WFPC2 drizzling notebook that was merged in PR #333. However, I receive a 404 error when trying to navigate to the HTML page. I do not see my new branch listed under the ci_manual_single_merge_generate workflow request, so it may need to be generated via another manual workflow? Thank you!

If I understand correctly, I believe we need to retrigger the CI to run on the drizzle_wfpc2.ipynb notebook from PR #333. I made a trivial change in this PR that has no consequence to the notebook, which will fire off the CI again and this should result in the generation of the HTML page.

@mrevalski
Copy link
Collaborator Author

Thank you! Can you or @haticekaratay merge this request into main as your time allows? 🙂

Copy link
Collaborator

@gibsongreen gibsongreen left a comment

Choose a reason for hiding this comment

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

This PR is a 1 line change to update the file format type so we can properly render the generated HTML file, ready to merge!

@gibsongreen gibsongreen merged commit c54c457 into spacetelescope:main Dec 23, 2024
5 checks passed
@mrevalski mrevalski deleted the fix-html-link branch December 23, 2024 15:31
@mrevalski
Copy link
Collaborator Author

mrevalski commented Dec 31, 2024

Hi @gibsongreen, I checked the HST Notebooks DrizzlePac Readme page and I am not seeing this update. Currently, hovering over the "Drizzling new WFPC2 flt data products" text still shows it linked to the *.ipynb instead of the *.html. Do you have suggestions on how to resolve this? I already ran the ci_manual_html_deploy without any affect. Thanks for your time and help!

@gibsongreen
Copy link
Collaborator

Hello @mrevalski, I am attaching a link to the ReadMe for the DrizzlePac notebooks on main:
https://raw.githubusercontent.com/spacetelescope/hst_notebooks/refs/heads/main/notebooks/DrizzlePac/README.md

This is where the update we made the update for .ipynb to .html, and I do see this updates. However, I do see the .iypnb when viewing the link. This PR with a small change to the notebook has worked in other repositories with the same issue, so I will check first if there is anything additional that needs to be done for hst_notebooks, and if that does not resolve this I will look into the html generation for this notebook.

@mrevalski
Copy link
Collaborator Author

Thank you for your assistance @gibsongreen. We are planning to advertise this revised notebook in a Space Telescope Announcement Newsletter (STAN) article led by @mackjenn this Friday, as well as at AAS next week, so this is relatively high priority for our team. Please let us know if we can be of help, or if you require any additional information. Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants