[afaisst]

I make this PR to start the review of the new SPHEREx all-sky HiPS cube notebook.

The notebook runs and everything is in place. @bsipocz might have to check the underlying MD code so that everything fits.

There are some things to be done after the SPHEREx all-sky mosaic is going online:

• HiPS URL needs to be changed from the test to the public one. It is currently using the test HiPS mosaic (https://loi.irsakubedev.ipac.caltech.edu/data/hips/SPHEREx/sample_hips_20261012)
• The Firefly client needs to be change to the public one. Currently using http://spherexdev2.ipac.caltech.edu:8090/firefly.

Link to notebook: https://github.com/Caltech-IPAC/irsa-tutorials/blob/spherex_hips_notebook_faisst/tutorials/spherex/spherex_hipscube.md

[jkrick]

This is a combo of my review along with an AI review. Code runs to completion for me with images etc. These are mostly easy comments to implement, so i will approve now, and I don't need to look at it again..

Will break CI or users:

1. Python version error (line 237): f"...{properties["hips_order_min"]}" puts double quotes inside a double-quoted f-string. That only works on Python 3.12 or newer, and CI also tests 3.10 and 3.11. Changing to properties['hips_order_min'] fixes it.
2. healpy not installed (line 73): it isn't in tutorial_requirements.txt, so the imports cell will fail in CI. Either add it, or switch to hpgeom, which is already listed (hpgeom.angle_to_pixel(nside, ra, dec, nest=True)).
3. URL built like a file path (line 259): os.path.join builds cube_url, which produces backslashes on Windows. Use an f-string or '/'.join.
   Correctness and efficiency:
4. Cube downloaded twice (line 301): [Table.read(cube_url](https://table.read(cube_url/), hdu="SPECTRAL_CHANNELS") downloads the whole cube (about 300 MB) again just to get one small table. Read it from the fits.open call that's already there.
5. For the spectrum, consider labeling wavelengths from spec_channels instead of wavetab, since it's already loaded and describes the cube's own planes. Similarly, consider reading the flux unit from the cube's BUNIT header rather than hard-coding bunit=u.MJy/[u.sr](https://u.sr/).
6. Possible crash on the log-scale plot (line 583): it uses norm='log' with vmin set to the 5th percentile, which can be 0 or below for background-subtracted data. If so, plotting raises an error.
7. (Minor) The photometry table's plane column is 1-based (np.arange(cube.shape[0]) + 1), while layer_id, layer_view, and Firefly's cubeIdx are all 0-based. It doesn't affect the spectrum (wavelengths are attached by row order), but a reader matching phot_table['plane'] against layer_view would be off by one channel. Consider dropping the + 1 for consistency.
8. Narrative in section 4. would benefit from more of a description of what a HiPS cube is, ie., what is its shape, what data does it hold, how is this different from other SPHEREx data types, why does this data type exist in the first place, what are good use cases for it?
9. Definition of 'hits' map is needed, this is not language I am familiar with.

Housekeeping:
10. Not listed anywhere: the notebook isn't in toc.yml, the SPHEREx gallery (spherex.md), or notebook_metadata.yml, so it won't show up on the site.
11. Cleanup:
- Unused imports: copy, sigma_clipped_stats.
- The !pip install line lists reproject and astroquery, which are never used. It
leaves out healpy, requests and firefly_client.
- Section numbering jumps from 6.1 to 6.3, and 6.3 appears twice.
- There's an empty code cell at the end.
- Typos: "closeset", "Luckliy", "her", "Aromatric".
- explanatory supplement will need to be updated for DR1 location
- runtime: It took me 2min15sec to run.

[afaisst]

Thank you, @jkrick. Implemented all of these.

[afaisst]

New push, I see this as the final before the HiPS cubes are life.

• added @jkrick's comments
• added Section 6.4, which uses Firefly to show the spectrum.

[jaladh-singhal]

@afaisst firefly part looks good, it can use some minor improvements.

[jaladh-singhal]

You can reuse fc. You don't need to open another connection to Firefly viewer, consider removing this.

If you wish to clean state of your old viewer containing HiPS, before displaying spectrum, you can do fc.reinit_viewer() here. But I personally think showing HiPS and spectrum together is nice since Firefly composes different image/table/chart views anyways.

[afaisst]

but I still have the other instance open, I want to have two Firefly windows...

[jaladh-singhal]

Ah ok, then up to you - fine to leave it.

[jaladh-singhal]

You can directly use the fc from above. This will show your spectrum in the same viewer that was showing HiPS.

Suggested change

	fc_spec.show_table(file_input=phot_table_firefly,
	fc.show_table(file_input=phot_table_firefly,