Skip to content

[ENH] dataset reference layer with lazy sktime loaders - #170

Open
aryamanDutta wants to merge 1 commit into
sktime:mainfrom
aryamanDutta:enh-128-dataset-reference-layer
Open

aryamanDutta wants to merge 1 commit into
sktime:mainfrom
aryamanDutta:enh-128-dataset-reference-layer

Conversation

@aryamanDutta

Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Fixes #128.

What does this implement/fix? Explain your changes.

This PR introduces a lightweight dataset reference and registry layer in pycaret.datasets.

  • Adds DatasetReference to encapsulate dataset metadata and loading behavior.
  • Adds DatasetRegistry to register and retrieve dataset references.
  • Introduces lazy loaders for the sktime datasets airline, lynx, and uschange.
  • Integrates registered dataset loading into get_data() while preserving its existing signature and local-file/remote-CSV resolution behavior.
  • Handles tuple-based dataset loader results by converting them into a DataFrame.

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

No. This contribution introduces no new dependencies.

What should a reviewer concentrate their feedback on?

  • The design and scope of DatasetReference and DatasetRegistry.
  • The integration of registered dataset loaders with get_data().
  • Backward compatibility with existing dataset-loading behavior.

Did you add any tests for the change?

Yes. Added tests covering dataset reference loading, registry operations, tuple-to-DataFrame conversion, missing loaders and dependencies, loader errors, local-file precedence, custom addresses, and save_copy behavior.

Validation performed:

  • py -3.14 -m pytest tests/test_datasets.py -v - 12 passed.
  • py -3.14 -m pytest tests/test_utils.py -v - 3 passed.

Any other comments?

The implementation is currently focused on introducing the reference/registry abstraction and integrating the selected sktime datasets. Feedback on the design and intended scope of the migration is welcome.

PR checklist

  • The PR title starts with [ENH], [MNT], [DOC], or [BUG].

@aryamanDutta

Copy link
Copy Markdown
Contributor Author

Hey @amotl , I've opened this PR for issue #128, introducing a lightweight DatasetReference and DatasetRegistry abstraction with lazy loaders for the sktime datasets airline, lynx, and uschange.

I've preserved the existing get_data() signature and added tests covering the new functionality and backward compatibility. All 12 dataset tests and 3 utility tests pass, and pre-commit checks are clean.

Could you please review the implementation and let me know if any changes are needed? Thank you for your guidance!

@amotl
amotl removed their request for review October 15, 2026 07:35
@amotl

amotl commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Thank you Aryaman. Because I am currently travelling, I'd like to defer the review to the other colleagues. 🙏

@amotl
amotl requested a review from WilliamJudge94 October 15, 2026 07:36
@aryamanDutta

Copy link
Copy Markdown
Contributor Author

No worries @amotl ..Safe journey !

@aryamanDutta

Copy link
Copy Markdown
Contributor Author

Hey @amotl, I investigated the two failing Python 3.10 Linux CI checks. Both fail with an ImportError for validate_data from sklearn.utils.validation, originating from daal4py/sklearn/ensemble/GBTDAAL.py.
The issue appears to stem from scikit-learn-intelex 2026.2.0 importing this function while the Python 3.10 environment resolves to scikit-learn 1.4.2. I also found the same failure on upstream PR #167, suggesting this is a pre-existing dependency compatibility issue rather than a regression introduced by this PR.
I haven't changed the implementation or added dependency pins to this PR, since the issue appears to affect the repository's CI environment more broadly.
Could you advise whether the CI dependency constraint should be addressed separately? Thanks!

@amotl

amotl commented Oct 18, 2026

Copy link
Copy Markdown
Member

Could you advise whether the CI dependency constraint should be addressed separately? Thanks!

Yes, please address that topic separately. Thank you!

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.

[MNT] Migrate datasets

2 participants