Skip to content

[draft] Output dataset#360

Draft
leoschwarz wants to merge 15 commits into
mainfrom
output-dataset
Draft

[draft] Output dataset#360
leoschwarz wants to merge 15 commits into
mainfrom
output-dataset

Conversation

@leoschwarz
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 1, 2025

📝 "TODO" Changes Detected

Summary: ➕ 5 "TODO"s added

➕ Added "TODO"s (5)

  • bfabric_app_runner/src/bfabric_app_runner/output_registration/annotation_table.py:30: # TODO maybe this should be a generic function somewhere, it's duplicated with input specs probably!
  • bfabric_app_runner/src/bfabric_app_runner/specs/outputs_spec.py:51: # TODO deprecate!
  • bfabric_app_runner/src/bfabric_app_runner/output_registration/annotation_table.py:50: # TODO the resource mapping has some challenges if it's not a model regarding mis-use non-canonical paths,
  • bfabric_app_runner/src/bfabric_app_runner/specs/outputs_spec.py:95: # TODO although the main scenario where this could be a problem is when store_folder_path is used and maybe we
  • bfabric_app_runner/src/bfabric_app_runner/output_registration/annotation_table.py:45: # TODO check if accessing store_entry_path like this is robust enough (see comment below)

This comment is automatically updated when "TODO" changes are detected.

# Conflicts:
#	.gitignore
#	bfabric_app_runner/src/bfabric_app_runner/output_registration/register.py
Wires `_save_annotations` to `bfabric.operations.dataset.create_dataset`
(landed in #500) instead of leaving it as a NotImplementedError stub.
Threads `workunit_definition` into the save path so the annotation
dataset is attached to the right workunit/container.

Tightens the annotation spec: adds an optional `name` field,
defaults `IncludeResourceRef.anchor` to `""`, drops the unused
`update_existing` from `IncludeDatasetRef`, and removes the
`UpdateExisting` import (which had created an import cycle).

Reintroduces `OutputsSpec.read_yaml` as a model-returning helper and
switches `register_outputs` and `cmd_validate_outputs_spec` to it.

Fixes a latent bug in `generate_output_table` where an empty
`include_tables` list crashed `pl.concat`; now raises a clear error
when both lists are empty.

Tests: adds `TestSaveAnnotations` with three mock-client cases,
covers `OutputsSpec.read_yaml` round-trip, updates the existing
anchor expectation from None to "".

"Update existing dataset" remains deferred — no `Workunit.output_dataset`
field exists today, so locating a prior dataset needs its own design.
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