Skip to content

Separate the act of resolving vs loading a dataset - #717

Open
ashkrisk wants to merge 1 commit into
mainfrom
ltm-ds/resolve
Open

Separate the act of resolving vs loading a dataset#717
ashkrisk wants to merge 1 commit into
mainfrom
ltm-ds/resolve

Conversation

@ashkrisk

@ashkrisk ashkrisk commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Make it so that the DataSetLoader is no longer responsible for deciding how to make a dataset's files available to the application. It remains responsible solely for resolving a dataset's files, which includes fetching them from a remote if required.

To faciliate this, the DataSetInfo object now knows about the underlying files which make up a dataset and is free to provide methods which allow the caller to access the dataset in a format of their choosing. In particular, this makes it possible to add methods to DataSetInfo which allow for accessing datasets without loading them completely into memory.

Summary of changes:

  • Introduces the DataSetFiles class for wrapping the base/query/gt paths of a dataset.
  • DataSetInfo no longer accepts a Supplier<Dataset>, instead accepting a DataSetFiles object.
  • DataSetInfo is responsible for converting DataSetFiles to an in-memory DataSet or other future formats.

Possible alternatives:

  • Introduce an additional abstraction layer responsible for converting DataSetFiles to a DataSet or similar instead of passing the responsibility onto DataSetInfo. This increases complexity without much immediate benefit, so this is left to future patches if and when it becomes necessary (if we start supporting HDF5 again, for example).

Make it so that the `DataSetLoader` is no longer responsible for
deciding how to make a dataset's files available to the application. It
remains responsible solely for resolving a dataset's files, which
includes fetching them from a remote if required.

To faciliate this, the `DataSetInfo` object now knows about the
underlying files which make up a dataset and is free to provide methods
which allow the caller to access the dataset in a format of their
choosing. In particular, this makes it possible to add methods to
`DataSetInfo` which allow for accessing datasets without loading them
completely into memory.

Summary of changes:
- Introduces the `DataSetFiles` class for wrapping the base/query/gt
  paths of a dataset.
- `DataSetInfo` no longer accepts a `Supplier<Dataset>`, instead
  accepting a `DataSetFiles` object.
- `DataSetInfo` is responsible for converting `DataSetFiles` to an
  in-memory `DataSet` or other future formats.

Possible alternatives:
- Introduce an additional abstraction layer responsible for converting
  `DataSetFiles` to a `DataSet` or similar instead of passing the
  responsiblity onto `DataSetInfo`. This increases complextiy without
  much immediate benefit, so this is left to future patches if and when
  it becomes necessary (if we start supporting HDF5 again, for example).
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Before you submit for review:

  • Does your PR follow guidelines from CONTRIBUTIONS.md?
  • Did you summarize what this PR does clearly and concisely?
  • Did you include performance data for changes which may be performance impacting?
  • Did you include useful docs for any user-facing changes or features?
  • Did you include useful javadocs for developer oriented changes, explaining new concepts or key changes?
  • Did you rebase your branch onto the latest main for regression testing and PR submission?
  • Did you trigger regression testing via Run Bench Main and review results?
  • Did you adhere to the code formatting guidelines (TBD)
  • Did you group your changes for easy review, providing meaningful descriptions for each commit?
  • Did you ensure that all files contain the correct copyright header?
  • Did you add documentation for this feature to the release notes directory?

If you did not complete any of these, then please explain below.

@ashkrisk
ashkrisk marked this pull request as ready for review August 31, 2026 04:40
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