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

Fix path coercion for pathlib-like interfaces (Pathy) #106

Closed
wants to merge 1 commit into from
Closed

Fix path coercion for pathlib-like interfaces (Pathy) #106

wants to merge 1 commit into from

Conversation

justindujardin
Copy link

Pathy inherited from pathlib.Path, so this always worked for saving spaCy models, but a new Pathy version that supports Python 3.12 will not inherit from pathlib.Path so this check fails and model saving goes with it.

Someone pointed out that the spaCy contributing doc specifies how things should work.

Code that interacts with the file-system should accept objects that follow the pathlib.Path API, without assuming that the object inherits from pathlib.Path. If the function is user-facing and takes a path as an argument, it should check whether the path is provided as a string. Strings should be converted to pathlib.Path objects.

I don't know if that extends to srsly, but I would think so.

There's more context on the Pathy 3.12 issue: justindujardin/pathy#106 (comment)

Pathy inherited from pathlib.Path so this always worked for saving spaCy models, but a new Pathy version that supports python 3.12 will not inherit from pathlib.Path so this check fails and model saving goes with it.

Someone pointed out that the [spaCy contributing doc](https://github.com/explosion/spaCy/blob/master/CONTRIBUTING.md#io-and-handling-paths) specifies how things should work. I'm not sure if that extends to srsly, but I would think so?

There's more context on the Pathy 3.12 issue: justindujardin/pathy#106 (comment)
@justindujardin justindujardin deleted the patch-1 branch January 11, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant