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

Unexpected behaviour of tokenizer.to_disk compared to documentation #5256

Open
lfiedler opened this issue Apr 4, 2020 · 2 comments
Open

Unexpected behaviour of tokenizer.to_disk compared to documentation #5256

lfiedler opened this issue Apr 4, 2020 · 2 comments

Comments

@lfiedler
Copy link
Contributor

@lfiedler lfiedler commented Apr 4, 2020

More precisely, if, for some reason, someone uses tokenizer.to_disk(path) as follows:

from spacy.util import get_lang_class
from tempfile import TemporaryDirectory
from pathlib import Path

tokenizer = get_lang_class("xx").Defaults.create_tokenizer()

with TemporaryDirectory() as temp_dir:
    tokenizer.to_disk(Path(temp_dir))

a PermissionError is raised, whereas

with TemporaryDirectory() as temp_dir:
    tokenizer.to_diks(Path(temp_dir) / "tokenizer.json")

works.

Judging from from this and the source code, path is opened as if it was a file.
The documentation of tokenizer.to_disk, however, describes it to require a path to a directory as argument. This seems to be inconsistent with its actual behaviour.

Your Environment

  • spaCy version: 2.2.4
  • Platform: Windows-10-10.0.18362-SP0
  • Python version: 3.7.7
  • Environment Information:
    • conda version: 4.8.3
    • conda-build version: 3.18.11
@svlandeg
Copy link
Member

@svlandeg svlandeg commented Apr 6, 2020

I think you're right, and the documentation needs to change.

@svlandeg svlandeg added the docs label Apr 6, 2020
@willpeppo
Copy link

@willpeppo willpeppo commented May 28, 2020

take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.