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

Better windows support #644

Merged
merged 35 commits into from
Sep 25, 2020
Merged

Better windows support #644

merged 35 commits into from
Sep 25, 2020

Conversation

lhoestq
Copy link
Member

@lhoestq lhoestq commented Sep 18, 2020 •
edited
Loading

There are a few differences in the behavior of python and pyarrow on windows.

For example there are restrictions when accessing/deleting files that are open

Fix #590

@lhoestq lhoestq changed the title Windows support [WIP] Better windows support Sep 18, 2020
@lhoestq lhoestq mentioned this pull request Sep 18, 2020
Closed
thomwolf
thomwolf reviewed Sep 18, 2020
View reviewed changes
Copy link
Member

@thomwolf thomwolf left a comment

Choose a reason for hiding this comment

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

Not sure about one modification (maybe it's related to the CI failure)

src/datasets/builder.py Outdated Show resolved Hide resolved
@lhoestq lhoestq mentioned this pull request Sep 22, 2020
Merged
@lhoestq
fix test_arrow_dataset tests on windows
c6fb176
@lhoestq
fix array tests
5b4092d
@lhoestq
fix arrow reader test
a10d959
@lhoestq
fix arrow writer test
e41c3f1
@lhoestq
fix test beam
d0bd5f3
@lhoestq
fix test caching
8c9ed71
@lhoestq
fix test builder
71c34ce
@lhoestq
use os.sep instead of regular slash
497f535
@lhoestq
fix missing require lib decorators
77792cc
@lhoestq
specify encoding when reqdig files
63b2fdf
@lhoestq
same for write
5650699
@lhoestq
fix dummy data url separator
1249457
@lhoestq
fix csv in sogou
73b7427
@lhoestq
fix cmrc dummy data
2bb4d51
@lhoestq
close writer before raising error while generating examples
8afeaa5
@lhoestq
fix web_question dummy data
4eda49a
@lhoestq
fix metric test
20a2a61
@lhoestq
fix dataset test
7e5bc81
@lhoestq
fix test search
8c51cfb
@lhoestq
fix test dataset dict
a5672d7
@lhoestq
fix dataset not compatible with windows
b930853
@lhoestq
fix test hf gcp
eb4f3cc
@lhoestq
fix tf export
3df94c4
@lhoestq
implement __del__ to fix permissions issues
2cb0828
@lhoestq
fix test metric
d05e9de
@lhoestq
fix weirdest bug I ever met (so far)
203d947
@lhoestq lhoestq force-pushed the fix-tests-on-windows branch from 58f4842 to 8eaadf6 Compare September 25, 2020 12:59
@lhoestq lhoestq changed the title [WIP] Better windows support Better windows support Sep 25, 2020
@lhoestq lhoestq requested a review from thomwolf September 25, 2020 13:02
@lhoestq
Copy link
Member Author

lhoestq commented Sep 25, 2020 •
edited
Loading

This PR is ready :)
It brings official support for windows.

Some tests AWSDatasetTest are failing.
This is because I had to fix a few datasets that were not compatible with windows.
These test will pass once they got merged on master :)

thomwolf
thomwolf approved these changes Sep 25, 2020
View reviewed changes
Copy link
Member

@thomwolf thomwolf left a comment

Choose a reason for hiding this comment

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

Honestly I mostly trust you here :)

.circleci/config.yml
@@ -26,6 +30,41 @@ jobs:
- run: pip install pyarrow==1.0.0
- run: HF_SCRIPTS_VERSION=master python -m pytest -sv ./tests/


run_dataset_script_tests_pyarrow_0p17_WIN:
Copy link
Member

Choose a reason for hiding this comment

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

Youpi!

@lhoestq lhoestq merged commit e86a2a8 into master Sep 25, 2020
@lhoestq lhoestq deleted the fix-tests-on-windows branch September 25, 2020 14:02
This was referenced Sep 25, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomwolf thomwolf thomwolf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The process cannot access the file because it is being used by another process (windows)
2 participants
@lhoestq @thomwolf