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 local filesystem remotes for projects #11762

Merged
merged 9 commits into from
Nov 29, 2022
Merged

Support local filesystem remotes for projects #11762

merged 9 commits into from
Nov 29, 2022

Conversation

adrianeboyd
Copy link
Contributor

@adrianeboyd adrianeboyd commented Nov 7, 2022 •
edited
Loading

Description

  • Fix support for local filesystem remotes for projects
    • Use FluidPath instead of Pathy to support both filesystem and remote paths
    • Create missing parent directories if required for local filesystem
  • Update pathy dependency to exclude older versions that aren't compatible with required smart_open version
  • Update docs to refer to Pathy instead of smart_open for project remotes
  • Add tests for local filesystem remotes

Types of change

Bug.

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@adrianeboyd
Support local filesystem remotes for projects
e9cd7e3 
* Fix support for local filesystem remotes for projects
  * Use `FluidPath` instead of `Pathy` to support both filesystem and
    remote paths
  * Create missing parent directories if required for local filesystem
  * Add a more general `_file_exists` method to support both `Pathy`,
    `Path`, and `smart_open`-compatible URLs
* Add explicit `smart_open` dependency starting with support for
  `compression` flag
* Update `pathy` dependency to exclude older versions that aren't
  compatible with required `smart_open` version
* Update docs to refer to `Pathy` instead of `smart_open` for project
  remotes (technically you can still push to any `smart_open`-compatible
  path but you can't pull from them)
* Add tests for local filesystem remotes
@adrianeboyd adrianeboyd added bug Bugs and behaviour differing from documentation feat / cli Feature: Command-line interface labels Nov 7, 2022
@adrianeboyd
Copy link
Contributor Author

It doesn't look like pathy supports python 3.11, so closing this for now in favor of #11750.

@adrianeboyd adrianeboyd closed this Nov 7, 2022
@adrianeboyd adrianeboyd mentioned this pull request Nov 7, 2022
Closed
@adrianeboyd
Copy link
Contributor Author

Reopening for testing because pathy has recently been updated.

@adrianeboyd adrianeboyd reopened this Nov 18, 2022
@adrianeboyd adrianeboyd marked this pull request as draft November 18, 2022 07:46
@adrianeboyd adrianeboyd marked this pull request as ready for review November 25, 2022 10:29
svlandeg
svlandeg approved these changes Nov 29, 2022
View reviewed changes
Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Looks good!

spacy/tests/test_cli.py Outdated Show resolved Hide resolved
@adrianeboyd adrianeboyd merged commit 1ebe7db into explosion:master Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justindujardin justindujardin justindujardin left review comments

@svlandeg svlandeg svlandeg approved these changes

Assignees
No one assigned
Labels
bug Bugs and behaviour differing from documentation feat / cli Feature: Command-line interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adrianeboyd @justindujardin @svlandeg