Skip to content

fix: propagate default target column warnings#443

Open
Saloni-0465 wants to merge 1 commit intosktime:mainfrom
Saloni-0465:fix/propagate-default-target-warning-clean
Open

fix: propagate default target column warnings#443
Saloni-0465 wants to merge 1 commit intosktime:mainfrom
Saloni-0465:fix/propagate-default-target-warning-clean

Conversation

@Saloni-0465
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

N/A

What does this implement/fix? Explain your changes.

This PR ensures that warnings generated during data conversion are surfaced in the top-level validation output.

Previously, if target_column was not provided, to_sktime_format would default to using the first column and store a warning inside the adapter metadata. However, this warning was not included in validation["warnings"], which is what downstream users (including agents) are expected to check.

With this change, any warnings generated during conversion are now propagated to the top-level validation report for both synchronous and asynchronous data loading. This makes the behavior more transparent and easier to debug.

Does your contribution introduce a new dependency? If yes, which one?

No.

What should a reviewer concentrate their feedback on?

  • Whether metadata-level conversion warnings should be merged into top-level validation
  • Whether the helper location in executor.py is appropriate
  • Test coverage for sync and async loading

Any other comments?

Added regression tests for sync and async load_data_source warning propagation.

Validation:

  • python -m compileall src/sktime_mcp/runtime/executor.py tests/test_data_validation_warnings.py
  • Manual sync and async checks confirm the default-target warning appears in validation["warnings"]

Note: local pytest exits with code -1 before output in this environment, so I verified behavior directly.

PR checklist

For all contributions
  • I've added myself to the list of contributors.
  • Optionally, I've updated CODEOWNERS.
  • I've added unit tests and made sure they pass locally where possible.

For new estimators

  • Not applicable

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