Skip to content

ENH: adding euclid clusters tutorial#284

Merged
bsipocz merged 14 commits intoCaltech-IPAC:mainfrom
bsipocz:ENH_euclid_clusters
Mar 25, 2026
Merged

ENH: adding euclid clusters tutorial#284
bsipocz merged 14 commits intoCaltech-IPAC:mainfrom
bsipocz:ENH_euclid_clusters

Conversation

@bsipocz
Copy link
Member

@bsipocz bsipocz commented Mar 11, 2026

This is a draft PR for now as the notebook runs way too long as is (2+hr locally)

TODO (that came up during making this PR, we may have other things left from the initial review):

  • fix figure captions
  • add metadata footer
  • make sure it runs and runs well within the confines of CI
  • cut up code cells, they are way too long and with little narrative in between

Extra things that are definitely not merging blockers, but what would make an improvement:

  • paper parsing for the table could be done with astropy, that may eliminate the need of introducing a new dependency (e.g. currently the notebook is failing due to the missing lxml library)
  • consider not using seaborn, as it is now a new dependency. If we were to use it in other notebooks, too then that's fine, but if it will be only this one, then I think we should reconsider to stay with mpl.

Resolves #282

Closes #283

@bsipocz bsipocz added the content: euclid Content related issues/PRs for notebooks with Euclid relevance label Mar 11, 2026
@jkrick
Copy link
Contributor

jkrick commented Mar 11, 2026

@bsipocz do you want me to look at how to shorten the runtime of this notebook? and also do you want me to look into adding narrative text? or do you already have a plan?

@bsipocz
Copy link
Member Author

bsipocz commented Mar 11, 2026

I would appreciate any help, thanks for offering.

@bsipocz
Copy link
Member Author

bsipocz commented Mar 12, 2026

@jkrick - you should be able to directly push to this branch. Based on what you showed today I think will know how to do that, but please ping me if you need any help.

@bsipocz
Copy link
Member Author

bsipocz commented Mar 12, 2026

Runtime actually doesn't look as bad on CI, I'm not sure why I had more issues locally. That being said if there is a reasonable way, maybe we could still cut it back to ~half?

@jkrick
Copy link
Contributor

jkrick commented Mar 18, 2026

I sped up runtime with some low hanging fruit - by parallelizing the download and downsampling the images for display. Let me know if this isn't enough.

I'd prefer to do a major text wrangling in a separate PR, so I will make a separate issue for that, and please don't wait for it in this PR.

@bsipocz bsipocz force-pushed the ENH_euclid_clusters branch from 925a0d2 to bb0fdf4 Compare March 25, 2026 20:49
@bsipocz bsipocz force-pushed the ENH_euclid_clusters branch from bb0fdf4 to c330e46 Compare March 25, 2026 20:54
@bsipocz bsipocz added the GHA buildhtml Enable extra buildhtml job on GHA label Mar 25, 2026
@bsipocz
Copy link
Member Author

bsipocz commented Mar 25, 2026

We're hitting circleCI memory limits with the new notebook, so I add it to the skip execution list for now.

@bsipocz bsipocz marked this pull request as ready for review March 25, 2026 21:44
@bsipocz
Copy link
Member Author

bsipocz commented Mar 25, 2026

I'm going ahead and merge this PR. Jessica said above that she was planning on working on a follow-up PR and the remaining items are not blockers either.

@bsipocz bsipocz merged commit 9097a68 into Caltech-IPAC:main Mar 25, 2026
8 checks passed
@bsipocz bsipocz deleted the ENH_euclid_clusters branch March 25, 2026 22:25
github-actions bot pushed a commit that referenced this pull request Mar 25, 2026
ENH: adding euclid clusters tutorial 9097a68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

content: euclid Content related issues/PRs for notebooks with Euclid relevance GHA buildhtml Enable extra buildhtml job on GHA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add description of Euclid clusters notebook Release Euclid clusters notebook

4 participants