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

Terminology: deprecated vs obsolete #7621

Merged
merged 2 commits into from
Apr 12, 2021
Merged

Terminology: deprecated vs obsolete #7621

merged 2 commits into from
Apr 12, 2021

Conversation

BramVanroy
Copy link
Contributor

Typically, deprecated is used for functionality that is bound to become unavailable but that can still be used. Obsolete is used for features that have been removed. In E941, I think what is meant is "obsolete" since loading a model by a shortcut simply does not work anymore (and throws an error). This is different from downloading a model with a shortcut, which is deprecated but still works.

In light of this, perhaps all other error codes should be checked as well.

Types of change

Change in error code

Checklist

  • I have submitted the spaCy Contributor Agreement.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

Typically, deprecated is used for functionality that is bound to become unavailable but that can still be used. Obsolete is used for features that have been removed. In E941, I think what is meant is "obsolete" since loading a model by a shortcut simply does not work anymore (and throws an error). This is different from downloading a model with a shortcut, which is deprecated but still works.

In light of this, perhaps all other error codes should be checked as well.
@svlandeg svlandeg added the feat / ux Feature: User experience, error messages etc. label Apr 1, 2021
@svlandeg
Copy link
Member

svlandeg commented Apr 12, 2021

Hi Bram,

Thanks for the PR. You're right that we haven't been using the terminology entirely consistently.
The changes you propose are fine and can be merged. I've also gone through the other warning/error codes, and I think those are all OK, because many of the functionality is in fact deprecated but not yet removed/obsolete.

I've made a few additional small changes to some documentation, but then I think this can be merged.

@svlandeg svlandeg merged commit ed561cf into explosion:master Apr 12, 2021
@BramVanroy BramVanroy deleted the patch-1 branch April 12, 2021 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat / ux Feature: User experience, error messages etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants