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.

Sign up for GitHub

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

Jump to bottom

Support paths as strings in public fetching API #453

Merged
merged 11 commits into from
Feb 17, 2023
Merged

Support paths as strings in public fetching API #453

merged 11 commits into from
Feb 17, 2023

Conversation

LilianBoulard
Copy link
Member

  • Adds support for paths as strings in the public fetching API
  • Uses cleaner temporary directories in the fetching test

@LilianBoulard LilianBoulard added the enhancement New feature or request label Jan 6, 2023
@LilianBoulard LilianBoulard self-assigned this Jan 6, 2023
@LilianBoulard LilianBoulard mentioned this pull request Jan 6, 2023
Closed
@LilianBoulard
Copy link
Member Author

LilianBoulard commented Jan 6, 2023 •
edited

Changelog entry taken care of by #432

Edit: nevermind, I'll add the PR number to it

jovan-stojanovic
jovan-stojanovic reviewed Jan 9, 2023
View reviewed changes
Copy link
Member

@jovan-stojanovic jovan-stojanovic left a comment

Choose a reason for hiding this comment

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

It seems there is a problem with the fetch_openml function and the temporary directory where you store downloaded files..

@LilianBoulard LilianBoulard added this to the 0.4.0 release milestone Jan 9, 2023
@LilianBoulard
Merge branch 'main' of https://github.com/dirty-cat/dirty_cat into st…
75ce2d9 
…r_paths_fetching

# Conflicts:
#	dirty_cat/datasets/tests/test_fetching.py
@GaelVaroquaux
Copy link
Member

Darn, there are test failures, and they seem related to this PR

@LilianBoulard
Copy link
Member Author

Yes, and it's system-specific, which is very annoying to debug... I'll try and tackle that today

@LilianBoulard LilianBoulard mentioned this pull request Feb 13, 2023
Merged
@LilianBoulard
Merge branch 'main' of https://github.com/dirty-cat/dirty_cat into st…
da881f3 
…r_paths_fetching

# Conflicts:
#	CHANGES.rst
#	dirty_cat/datasets/_fetching.py
#	dirty_cat/datasets/tests/test_fetching.py
@LilianBoulard
Remove unnecessary path conversion in test
83fd7f8
@LilianBoulard
Merge branch 'main' of https://github.com/dirty-cat/dirty_cat into st…
c14d855 
…r_paths_fetching
@LilianBoulard LilianBoulard merged commit 72be1af into skrub-data:main Feb 17, 2023
@LilianBoulard LilianBoulard deleted the str_paths_fetching branch February 17, 2023 15:11
@LilianBoulard LilianBoulard restored the str_paths_fetching branch February 17, 2023 15:12
LilianBoulard added a commit that referenced this pull request Feb 17, 2023
@LilianBoulard
Revert "Support paths as strings in public fetching API (#453)"
0a5580f 
This reverts commit 72be1af.
@LilianBoulard LilianBoulard mentioned this pull request Feb 17, 2023
Merged
LilianBoulard added a commit that referenced this pull request Feb 17, 2023
@LilianBoulard
Revert "Support paths as strings in public fetching API (#453)" (#498)
99ede6f 
This reverts commit 72be1af.
@LilianBoulard LilianBoulard removed this from the 0.4.0 release milestone Feb 20, 2023
@LilianBoulard LilianBoulard added this to the 0.4.1 milestone Feb 20, 2023
LilianBoulard added a commit that referenced this pull request Feb 20, 2023
@LilianBoulard
Revert "Revert "Support paths as strings in public fetching API (#453)…
c1362d9 
…" (#498)"

This reverts commit 99ede6f.
@LilianBoulard LilianBoulard mentioned this pull request Feb 20, 2023
Merged
@LilianBoulard LilianBoulard deleted the str_paths_fetching branch February 21, 2023 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jovan-stojanovic jovan-stojanovic jovan-stojanovic left review comments

Assignees

@LilianBoulard LilianBoulard

Labels
enhancement New feature or request
Projects
None yet
Milestone
0.4.1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@LilianBoulard @GaelVaroquaux @jovan-stojanovic