Skip to content

add: DashPVA notebooks — Tools Tour, RSM Gridder, and a notebooks index - #159

Merged
Osayi-ANL merged 7 commits into
mainfrom
docs/notebooks
Sep 16, 2026
Merged

Osayi-ANL merged 7 commits into
mainfrom
docs/notebooks

Conversation

@Osayi-ANL

Copy link
Copy Markdown
Collaborator

Adds a notebooks set for the DashPVA tools, and reorganizes the existing one.

  • DashPVA_Tools_Tour.ipynb (new) — tour of the toolset.
  • RSM_Gridder.ipynb (new) — building a gridded reciprocal-space volume, including a 3D volume render of the live grid.
  • README.md (new) — index for the notebooks directory.
  • DashAnalysis_Quickstart.ipynb — moved up from notebooks/dash_analysis/ to notebooks/, and revised; the now-redundant notebooks/dash_analysis/README.md is folded into the new index.

Notebooks only — nothing under src/, so no version bump.

@Osayi-ANL
Osayi-ANL requested a review from pecomyint September 9, 2026 15:17

@pecomyint pecomyint left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for keeping this PR isolated to notebooks and documentation. I am requesting a few focused corrections before merge. In the RSM notebook, pass an explicit frame (for example, frame=0) to RSMConverter.get_sample_and_detector_circles, because the current call does not match the required API. Please construct MaskManager with a temporary notebook-specific directory; the default can load a user’s active mask, and clear_mask() may delete masks/active_mask.npy. Expand user paths before passing them to h5py, and clear the stale ladida13.h5 notebook outputs. Finally, either guard the Quickstart load_data("your_data_file.h5") call or revise the README claim that the notebooks run top-to-bottom without external data. Please also resolve the Python 3.12 CI failure. These changes preserve the notebook-only scope while making the examples safe and reproducible.

@pecomyint
pecomyint self-requested a review September 11, 2026 23:04
@Osayi-ANL

Copy link
Copy Markdown
Collaborator Author

All six points are in, as three commits.

  • get_sample_and_detector_circles now gets frame=0. It also returns four values, not two, so the old call would have raised ValueError even with the frame added. The cell now unpacks sc_dir, sc_pos, dc_dir, dc_pos.
  • MaskManager in both notebooks now uses a tempfile.mkdtemp directory, so clear_mask() cannot touch masks/active_mask.npy.
  • User paths are expanded with os.path.expanduser in all three notebooks. That was also the cause of the ladida13.h5 errors, since h5py does not expand ~.
  • Those stale outputs are cleared and the execution counts reset.
  • The Quickstart load_data call is guarded with an existence check, so the README claim about running top to bottom is now true rather than walked back.
  • Python 3.12 CI is green, along with Ruff and 3.11.

@pecomyint pecomyint left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

APPROVED, BUT PLEASE FIX THESE BEFORE MERGING: guard or skip all later Quickstart cells when data is unavailable, because it still cannot run top-to-bottom without a real file. ALSO CLEAR THE SIX SAVED OUTPUTS AND FIVE EXECUTION COUNTS STILL PRESENT IN RSM_Gridder.ipynb.

…EADME index

change: flatten notebooks/dash_analysis/ up to notebooks/ so the three sit together

add: figures to the two notebooks that were otherwise all print() — memory-budget curve, built-volume projection with a log intensity histogram, nested 3D isosurfaces, live-grid max projections, mask before/after, and Q-space coverage; the ones that depend on a real scan are guarded and print a hint instead of raising
…he notebook runs top-to-bottom without a scan file — cell 8 set data=None but the 15 cells after it still dereferenced it
…ipynb — committed outputs bloat the diff and go stale against the code
@Osayi-ANL
Osayi-ANL merged commit 862109f into main Sep 16, 2026
3 checks passed
@Osayi-ANL
Osayi-ANL deleted the docs/notebooks branch September 16, 2026 16:09
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