Skip to content

feat!: keyword-only optional parameters and public types of the storage, loader and sensor APIs - #257

Open
janickm wants to merge 1 commit into
mainfrom
stack/v20-rc-cleanup/01-keyword-only-exports
Open

janickm wants to merge 1 commit into
mainfrom
stack/v20-rc-cleanup/01-keyword-only-exports

Conversation

@janickm

@janickm janickm commented Nov 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

First of six PRs that clean up the public API before v20.0.0-rc.1, after which the 20.0 API is frozen. This one makes optional parameters keyword-only across the storage, loader, writer and sensor model APIs, and exports from the public packages every type those APIs take and return.

Positional calls silently bind to the wrong parameter whenever a parameter is added in between. For example, a v19-style SequenceComponentGroupsReader(paths, True, 1 << 20, 8) now passes 8 as the index tail retention instead of max_threads. Making these parameters keyword-only closes that class of bug before the freeze.

Stack (merge from the top to land all six at once):

  1. Keyword-only parameters and public exports (this PR)
  2. Storage contract and Group / Array API
  3. Sensor models
  4. Geometry cleanup
  5. Loaders, V4 components, and ValueError for user-facing validation
  6. Converter default, wheel requirements, tools and docs (upgrading guide)

Changes

  • Keyword-only after the leading argument(s), in both the sync constructor and its open_async twin:
    • SequenceComponentGroupsReader(component_group_paths, *, ...), SequenceLoaderV4(reader, *, ...), SequenceLoaderV4.open_async(component_group_paths, *, ...)
    • IndexedTarArchive(itar_path, *, ...), IndexedTarZarrStore(itar_path, *, ...)
    • SequenceComponentGroupsWriter(..., *, store_type, zarr_format) and .from_reader(...)
    • PointCloudsComponent.Writer(..., *, attribute_schemas), store_pc(xyz, reference_frame_id, reference_frame_timestamp_us, *, ...), CameraLabelsComponent.Writer.store_label(data, timestamp_us, *, ...)
    • RayBundleSensorPointCloudsSourceAdapter(sensor, *, return_index, promote_generic_data) and its async twin
    • Camera models: (parameters, device, dtype, *, tuning...) for FTheta, OpenCV fisheye and OpenCV pinhole
    • RowOffsetStructuredSpinningLidarModel(parameters, device, dtype, *, ...): device / dtype move directly after parameters, matching the camera models
    • The optional arguments of the camera pixels_to_world_rays_* / image_points_to_world_rays_* methods and of the lidar elements_to_world_rays_shutter_pose
  • New public exports:
    • ncore.data.stores: ZarrStore, NodeCache, IndexTailRetention, ZarrFormat, Selection, FillValue, Attributes, BloscCname, BloscShuffle
    • ncore.data: RayBundleSensorProtocol, AsyncRayBundleSensorPointCloudsSourceAdapter, FOV
    • ncore.data.v4: BaseSensorComponentReader, BaseSensorComponentWriter, BaseRayBundleSensorComponentReader, BaseRayBundleSensorComponentWriter, IndexTailRetention
  • The type aliases are documented on the API pages.
  • ZarrStore is the store base class of the installed zarr-python at runtime (was object), so isinstance works.

Testing

  • Each package's test.py checks the exact __all__ and that every export is the implementation object.
  • Rejection tests for positional use of each keyword-only group. Removing the * makes them fail; the reader test reproduces the v19 bug above (unsupported index tail retention 8).
  • bazel test //... --config=no-gpu green on all four interpreters, bazel build //... (ty), docs with warnings as errors, and bazel run //:format.check clean. No in-repo caller passed these parameters positionally.

Upgrading

  • Optional parameters of the APIs above must be passed by keyword; positional calls raise TypeError. For example, IndexedTarZarrStore(path, "w") becomes IndexedTarZarrStore(path, mode="w").
  • RowOffsetStructuredSpinningLidarModel: positional device / dtype now come directly after parameters.
  • Import the newly exported types from the public packages instead of ncore.impl.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

Checklist

  • I have read the Contributing Guidelines
  • My commits follow Conventional Commits format
  • My commits are GPG-signed
  • I have added tests that prove my fix is effective or my feature works
  • New and existing tests pass locally (bazel test //...)
  • Code is formatted (bazel run //:format)
  • I have updated documentation as needed
  • My changes include SPDX license headers on all new files

…ge, loader and sensor APIs

The optional parameters of the sequence reader, loader and writer, the
point-cloud and camera-label writers, the point-cloud source adapters of
ray bundle sensors, the indexed tar archive and store, the tuning
parameters of the camera and lidar models and the optional arguments of
their world-ray unprojections are now keyword-only. Positional calls silently bound arguments to the wrong
parameter whenever a parameter was added in between: e.g., a positional
max_threads of SequenceComponentGroupsReader was taken as the index tail
retention.

The public packages now export the types their APIs take and return:
ncore.data.stores exports ZarrStore, NodeCache, IndexTailRetention,
ZarrFormat, Selection, FillValue, Attributes, BloscCname and BloscShuffle;
ncore.data exports RayBundleSensorProtocol,
AsyncRayBundleSensorPointCloudsSourceAdapter and FOV; ncore.data.v4 exports
the base sensor component readers and writers and IndexTailRetention. The
type aliases are documented on the API pages. ZarrStore is the store base
class of the installed zarr-python version at runtime (it was object).

BREAKING CHANGE: the following parameters are keyword-only:
SequenceComponentGroupsReader(component_group_paths, *, ...) and its
open_async; SequenceLoaderV4(reader, *, ...) and
SequenceLoaderV4.open_async(component_group_paths, *, ...);
IndexedTarArchive(itar_path, *, mode, index_tail_read_size,
remote_range_reads) and IndexedTarArchive.open_async(itar_path, *, ...);
IndexedTarZarrStore(itar_path, *, mode, index_tail_read_size,
remote_range_reads) and IndexedTarZarrStore.open_async(itar_path, *, ...);
FThetaCameraModel and OpenCVFisheyeCameraModel(parameters, device, dtype, *,
newton_iterations, min_2d_norm); OpenCVPinholeCameraModel(parameters,
device, dtype, *, max_normalized_radius_squared);
RowOffsetStructuredSpinningLidarModel(parameters, device, dtype, *,
angles_to_columns_map_resolution_factor, angles_to_columns_map_dtype,
angles_to_columns_map_init, fov_eps_factor), where device and dtype now
directly follow the parameters (they came after the angles_to_columns_map
arguments).
SequenceComponentGroupsWriter(..., generic_meta_data, *, store_type,
zarr_format) and SequenceComponentGroupsWriter.from_reader(...,
sequence_reader, *, store_type, zarr_format);
PointCloudsComponent.Writer(..., coordinate_unit, *, attribute_schemas),
PointCloudsComponent.Writer.store_pc(xyz, reference_frame_id,
reference_frame_timestamp_us, *, attributes, generic_data,
generic_meta_data) and CameraLabelsComponent.Writer.store_label(data,
timestamp_us, *, generic_meta_data);
RayBundleSensorPointCloudsSourceAdapter(sensor, *, return_index,
promote_generic_data) and its asynchronous twin; the camera_rays /
sensor_rays, return_T_sensor_worlds and return_timestamps arguments of
the CameraModel pixels_to_world_rays_* / image_points_to_world_rays_*
methods and of the lidar elements_to_world_rays_shutter_pose.
@janickm
janickm added this pull request to stack #263 November 14, 2026 06:56
@janickm janickm self-assigned this Nov 15, 2026

This branch has not been deployed

No deployments
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.

1 participant