Skip to content

Conversation

@mfeurer
Copy link
Collaborator

@mfeurer mfeurer commented Apr 14, 2021

by only loading necessary datasets

by only loading necessary datasets
@mfeurer mfeurer requested a review from PGijsbers April 14, 2021 10:58
Copy link
Collaborator

@PGijsbers PGijsbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to just work with a lazy-load property?
e.g.

self._titanic = None

@property
def titantic(self):
     if self._titanic is None:
         self._titanic = download..
     return self._titanic

I guess the fact that it requires less changes is a bit moot at this point, but it does make more use of code re-use (which is more maintainable and less error prone).

@mfeurer
Copy link
Collaborator Author

mfeurer commented Apr 20, 2021

I guess the fact that it requires less changes is a bit moot at this point, but it does make more use of code re-use (which is more maintainable and less error prone).

Yes, I agree on code reuse. It'll also be easier if we at one point move to pytest.

@mfeurer mfeurer requested a review from PGijsbers April 20, 2021 18:15
Copy link
Collaborator

@PGijsbers PGijsbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly :) Conversion to fixtures should be no issue at all.

@PGijsbers PGijsbers merged commit 6b71981 into develop Apr 21, 2021
@PGijsbers PGijsbers deleted the speed_up_dataset_tests branch April 21, 2021 09:35
PGijsbers pushed a commit to Mirkazemi/openml-python that referenced this pull request Feb 23, 2023
* Speed up dataset unit tests

by only loading necessary datasets

* Revert "Speed up dataset unit tests"

This reverts commit 861b52d.

* address suggestions from Pieter
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.

3 participants