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

Raise error in spacy package when model name is not a valid python identifier #10192

Merged
merged 13 commits into from
Feb 10, 2022
Merged

Raise error in spacy package when model name is not a valid python identifier #10192

merged 13 commits into from
Feb 10, 2022

Conversation

pmbaumgartner
Copy link
Contributor

@pmbaumgartner pmbaumgartner commented Feb 1, 2022

This PR affects spacy package. It adds an error when the --name argument (or model name from the meta.json) is not a valid python identifier.

The model name needs to be a valid model identifier so that it can be included as module in the spacy_models entry point from the setup.py file.

Prior to this PR, users would get the following message if trying to package a model with an invalid name:

error in en_spacy-sst-cpu-efficiency-128 setup command: ("EntryPoint must be in 'name=module:attrs [extras]' format", 'en_spacy-sst-cpu-efficiency-128 = en_spacy-sst-cpu-efficiency-128')

Description

Example output:

$ python -m spacy package model-best output/ --name CAPS-HYPHEN-NAME
ℹ Building package artifacts: sdist

✘ Model name ('CAPS-HYPHEN-NAME') is not a valid python identifier.
This is required to correctly package the pipeline so it can be imported as a
module.
Valid characters for identifiers are ASCII A-Z, a-z, _ (underscore)
and 0-9 (except as the first character)

Types of change

enhancement

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.

spacy/cli/package.py Outdated Show resolved Hide resolved
spacy/cli/package.py Outdated Show resolved Hide resolved
spacy/cli/package.py Outdated Show resolved Hide resolved
spacy/cli/package.py Outdated Show resolved Hide resolved
spacy/cli/package.py Outdated Show resolved Hide resolved
@pmbaumgartner pmbaumgartner added the feat / cli Feature: Command-line interface label Feb 3, 2022
@pmbaumgartner
Copy link
Contributor Author

Here's a summary of the three new errors/warnings:

Invalid module name

python -m spacy package model-best output/ --name invalid-module
ℹ Building package artifacts: sdist

✘ Model name ('invalid-module') is not a valid module name. This is
required so it can be imported as a module.
We recommend names that use ASCII A-Z, a-z, _ (underscore), and 0-9. 
For specific details see:
https://docs.python.org/3/reference/lexical_analysis.html#identifiers

Invalid package name

python -m spacy package model-best output/ --name invalid_päckage 
ℹ Building package artifacts: sdist

✘ Model name ('invalid_päckage') is not a permitted package name. This
is required to correctly load the model with spacy.load.
We recommend names that use ASCII A-Z, a-z, _ (underscore), and 0-9. 
For specific details see: https://www.python.org/dev/peps/pep-0426/#name

Underscore Run

python -m spacy package model-best output/ --name multiple______underscore  --build wheel
[...]
✔ Successfully created binary wheel
output/en_multiple______underscore-0.0.0/dist/en_multiple_underscore-0.0.0-py3-none-any.whl
⚠ Model name ('en_multiple______underscore') contains a run of
underscores. Runs of underscores are not significant in installed package
names.

validating wheel file name exists:

$ ls output/en_multiple______underscore-0.0.0/dist/en_multiple_underscore-0.0.0-py3-none-any.whl
output/en_multiple______underscore-0.0.0/dist/en_multiple_underscore-0.0.0-py3-none-any.whl

spacy/tests/test_cli.py Outdated Show resolved Hide resolved
@adrianeboyd adrianeboyd merged commit ee662ec into explosion:master Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat / cli Feature: Command-line interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants