Skip to content

fix: propagate conversion warnings and implement strict exogenous validation#442

Closed
Saloni-0465 wants to merge 2 commits intosktime:mainfrom
Saloni-0465:fix/propagate-default-target-warning
Closed

fix: propagate conversion warnings and implement strict exogenous validation#442
Saloni-0465 wants to merge 2 commits intosktime:mainfrom
Saloni-0465:fix/propagate-default-target-warning

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

@Saloni-0465
Copy link
Copy Markdown
Contributor Author

Closing this PR because the branch accidentally included an unrelated earlier commit. Reopened a clean PR from origin/main here: #443

@Saloni-0465 Saloni-0465 closed this May 4, 2026
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