Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace integration test that uses the iris demo data #352

Closed
npatki opened this issue Apr 4, 2024 · 0 comments · Fixed by #357
Closed

Replace integration test that uses the iris demo data #352

npatki opened this issue Apr 4, 2024 · 0 comments · Fixed by #357
Assignees
Labels
internal The issue doesn't change the API or functionality
Milestone

Comments

@npatki
Copy link
Contributor

npatki commented Apr 4, 2024

Problem Description

The test_tvae integration test currently uses the iris demo dataset from scikit learn.

iris = datasets.load_iris()

However, as of the latest merge, scikit-learn is no longer a dependency of CTGAN. So we shouldn't be using it for any testing. It is better to replace this dataset with a different demo (or potentially a hard-coded one).

Additional context

If we don't require skicit-learn anymore, then why is this integration test still passing? That's because CTGAN requires RDT to run, and RDT requires scikit-learn.

In any case, we'd like to clean up dependencies within each library. Since the functionality of CTGAN doesn't directly need scikit-learn, we should not have it referenced in this repo.

@npatki npatki added the internal The issue doesn't change the API or functionality label Apr 4, 2024
@amontanez24 amontanez24 added this to the 0.10.0 milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal The issue doesn't change the API or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants