Codereview

[epifanio]

codereview: robustness, security fixes, and axis control UX

Security

• js_util.py: Replace Javascript class (which used eval() to execute
  arbitrary JS) with Redirector, a safe ReactiveHTML component that uses
  window.location.assign() directly. No code is evaluated; the URL is
  reset to '' after the redirect fires to allow reuse.

Bug fixes

• TSP/main.py – plottable-variable selection: Replace the fragile
  heuristic (coord-in-dims intersection) with get_plottable_vars() from
  utility.py. The old approach missed variables in DSG (Discrete Sampling
  Geometry) datasets where the observation dimension has no registered
  coordinate, and incorrectly included coordinate/metadata variables
  (latitude, longitude, bottomDepth, profileIdAll, QC flags, etc.).

• TSP/main.py – dimension selector: Replace list(ds[result].indexes)
  with get_axis_candidates(), which scans named indexes, auxiliary
  1-D coordinates, and coordinate-like data variables (depth, pressure,
  etc.) while excluding spatial metadata (lat, lon, altitude, station,
  …) via a configurable AXIS_BLACKLIST. Previously the selector was
  populated with all data variables, making it unusable on real datasets.

• TSP/main.py – dimension switching: Fix the non-timeseries branch of
  plot() where x was incorrectly set to var instead of dimension,
  causing hvplot to ignore the dimension selection entirely. Now always
  sets axis_arguments['x'] = dimension and removes any stale 'y' key.
  For DSG datasets where the dimension variable is not registered as a
  coordinate on the target variable, falls back to a Dataset-level
  hvplot.line(x=dimension, y=var) call using a minimal sub-dataset.

• TSP/main.py – y-axis inversion: Guard ds[dimension].attrs lookup with
  if dimension in ds: to avoid KeyError when dimension is a raw
  dimension name (e.g. obs) with no associated variable.

• TSP/main.py – on_var_select(): Fix crash where dimension_group.options
  was populated with list(ds[result].indexes) (result is a list, not a
  string key) instead of axis candidates for the selected variable. Rebuild
  axis candidates and reset dimension_group.value to the first option
  whenever the variable changes.

• TSP/main.py – export resampling logic: Fix inverted boolean in
  export_selection() that caused the resampler flag to always be False
  even when a non-raw frequency was selected.

• TSP/main.py – show/hide buttons: Fix show_hide_widget() so that
  opening the metadata panel closes the downloader and vice-versa, by
  accepting explicit hide= and reveal= arguments via functools.partial.

• TSP/utility.py – validate_opendap(): Use a context manager
  (with xr.open_dataset(...) as ...) to avoid leaking file handles.

• TSP/utility.py – on_session_destroyed(): Remove invalid
  del ds / del plot_widget calls that referenced variables not in
  scope, which would raise UnboundLocalError on every session teardown.

• TSP/utility.py – load_data() ERDDAP branch: Replace three
  separate xr.open_dataset() calls (each opening a new connection)
  with a single context-managed open.

• ncapp/app/main.py: Fix from http.client import HTTPException (wrong
  module; HTTPException is from FastAPI). Rename duplicate index()
  route handlers to tsp() and trj() to avoid silent handler shadowing.
  Remove duplicate FastAPI() instantiation.

New features

• TSP/main.py – axis control: Add Invert Y-axis and Swap axes
  checkboxes to the toolbar. Invert Y-axis combines with the existing
  CF-attribute / dimension-name auto-detection (OR logic) so users can
  force inversion on any dataset. Swap axes applies invert_axes=True
  to the HoloViews plot for the classic oceanographic profile orientation
  (depth on y, variable on x). Both flags are propagated through all
  variable/dimension/frequency change callbacks so state is preserved
  across interactions.

• TSP/utility.py: Extract and export get_plottable_vars(),
  get_axis_candidates(), is_plottable(), safe_check_var(), and
  AXIS_BLACKLIST so the variable- and axis-selection logic is reusable
  and testable in isolation.

• TSP/utility.py: Add build_metadata_widget() returning a plain
  Div instead of a nested pn.Row/Column, reducing layout nesting
  and fixing the widget-width issue that caused metadata to overflow.

• trj/main.py: Add metadata panel (dataset global attributes) to the
  trajectory viewer, wired through build_metadata_widget() from the
  shared utility module. Add on_load() callback to hide the metadata
  panel after the page finishes loading (workaround for Panel layout
  width initialisation). Switch layout from pn.Column to pn.GridBox
  inside pn.Row to keep the map and plot side-by-side with the
  metadata panel.

Cleanup / code quality

• Replaced all print() debug statements in TSP/main.py with
  logger.info() / logger.warning() calls.
• Removed dead safe_check() local closure (superseded by
  safe_check_var() in utility.py).
• Removed a hardcoded example-data dict that was embedded in the
  export_selection() payload, causing exports to always target a fixed
  URL instead of the user's loaded dataset.
• Removed a dead BootstrapTemplate block that was never reachable.
• Replaced bare except: clauses with except Exception: throughout
  utility.py.
• docker/entrypoint.sh: Update panel serve command to only mount
  /TSP and /TRJ (removing stale /trj, /OGC_client, /seaice/*,
  /seaicemod/* mounts that no longer exist); rename /TRJ title from
  "trj" to "Trajectory".
• ncapp/app/utility.py: New file — Pydantic v2 models (FeatureType,
  FeatureTypeEnum, URLStr, OpendapURL) and guess_feature_type_from_data()
  helper extracted into a dedicated module for the FastAPI proxy service.