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

GH-37555: [Python] Update get_file_info_selector to ignore base directory #37558

Merged
merged 8 commits into from
Sep 14, 2023
Merged

GH-37555: [Python] Update get_file_info_selector to ignore base directory #37558

merged 8 commits into from
Sep 14, 2023

Conversation

AlenkaF
Copy link
Member

@AlenkaF AlenkaF commented Sep 5, 2023 •
edited
Loading

Rationale for this change

There has been some changes in the way fsspec lists the directories with new version 2023.9.0, see fsspec/filesystem_spec#1329, which caused our tests to start failing.

What changes are included in this PR?

This PR updates the get_file_info_selector in FSSpecHandler class to keep the behaviour of our spec.

Are there any user-facing changes?

No.

@github-actions
Copy link

github-actions bot commented Sep 5, 2023

⚠️ GitHub issue #37555 has been automatically assigned in GitHub to PR creator.

@AlenkaF
Copy link
Member Author

AlenkaF commented Sep 5, 2023

Conda builds are still using older version of fsspec.

@danepitkin
Copy link
Member

Should we add checks in the test for different fsspec versions?

@danepitkin
Copy link
Member

I suppose not, since our tests should always pull the latest dependencies.

@kou kou changed the title GH-37555: Update test_get_file_info_with_selector GH-37555: [Python] Update test_get_file_info_with_selector Sep 5, 2023
@AlenkaF
Copy link
Member Author

AlenkaF commented Sep 6, 2023

I suppose not, since our tests should always pull the latest dependencies.

Exactly. Maybe forcing the new version of fsspec for a while?

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 6, 2023
@AlenkaF AlenkaF marked this pull request as ready for review September 11, 2023 09:56
@chrisjordansquire chrisjordansquire mentioned this pull request Sep 11, 2023
Merged
danepitkin
danepitkin reviewed Sep 11, 2023
View reviewed changes
python/pyarrow/fs.py Outdated Show resolved Hide resolved
This was referenced Sep 12, 2023
Closed
Merged
danepitkin
danepitkin reviewed Sep 12, 2023
View reviewed changes
Copy link
Member

@danepitkin danepitkin left a comment

Choose a reason for hiding this comment

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

Overall LGTM! Thanks, @AlenkaF!

python/pyarrow/fs.py Outdated Show resolved Hide resolved
@h-vetinari
Copy link
Contributor

Sidenote: the title of this PR has nothing to do with its content anymore.

@AlenkaF AlenkaF changed the title GH-37555: [Python] Update test_get_file_info_with_selector GH-37555: [Python] Update get_file_info_selector to ignore base directory Sep 13, 2023
@AlenkaF AlenkaF mentioned this pull request Sep 13, 2023
Closed
@danepitkin
Copy link
Member

LGTM! Thanks @AlenkaF !

@AlenkaF
Copy link
Member Author

AlenkaF commented Sep 14, 2023

Decided to remove the test for issue #36983 as I am eager to get the AppVeyor builds working. Will add a test for it in a separate PR.

@AlenkaF
Copy link
Member Author

AlenkaF commented Sep 14, 2023

Tests passing, will merge.

@AlenkaF AlenkaF merged commit 396b475 into apache:main Sep 14, 2023
11 checks passed
@AlenkaF AlenkaF removed the awaiting committer review Awaiting committer review label Sep 14, 2023
@AlenkaF AlenkaF deleted the gh-37555-fsspec-failing-tests branch September 14, 2023 10:42
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 396b475.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

@h-vetinari h-vetinari mentioned this pull request Sep 25, 2023
Merged
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
@AlenkaF @loicalleyne
apacheGH-37555: [Python] Update get_file_info_selector to ignore base…
e0423ee 
… directory (apache#37558)

### Rationale for this change

There has been some changes in the way fsspec lists the directories with new version 2023.9.0, see fsspec/filesystem_spec#1329, which caused our tests to start failing.

### What changes are included in this PR?

This PR updates the `get_file_info_selector` in [FSSpecHandler](https://arrow.apache.org/docs/_modules/pyarrow/fs.html#FSSpecHandler) class to keep the behaviour of our spec.

### Are there any user-facing changes?

No.

* Closes: apache#37555

Authored-by: AlenkaF <[email protected]>
Signed-off-by: AlenkaF <[email protected]>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
@AlenkaF @dgreiss
apacheGH-37555: [Python] Update get_file_info_selector to ignore base…
94fca2a 
… directory (apache#37558)

### Rationale for this change

There has been some changes in the way fsspec lists the directories with new version 2023.9.0, see fsspec/filesystem_spec#1329, which caused our tests to start failing.

### What changes are included in this PR?

This PR updates the `get_file_info_selector` in [FSSpecHandler](https://arrow.apache.org/docs/_modules/pyarrow/fs.html#FSSpecHandler) class to keep the behaviour of our spec.

### Are there any user-facing changes?

No.

* Closes: apache#37555

Authored-by: AlenkaF <[email protected]>
Signed-off-by: AlenkaF <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danepitkin danepitkin danepitkin left review comments

@pitrou pitrou pitrou left review comments

Assignees
No one assigned
Labels
Component: Python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CI][Python] test_get_file_info_with_selector is failed on AppVeyor
4 participants
@AlenkaF @danepitkin @h-vetinari @pitrou