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

Set vectors.name when updating meta.json during training #3100

Merged
merged 2 commits into from
Dec 27, 2018

Conversation

jarib
Copy link
Contributor

@jarib jarib commented Dec 27, 2018

Description

This fixes #3093 for spacy train. Still need to check if it fixes spacy package for #3067, so it's a WIP.

Types of change

Bug fix

Checklist

  • I have submitted the spaCy Contributor Agreement.
  • 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.

@explosion-bot
Copy link
Collaborator

Hi @jarib, thanks for your pull request! 👍 It looks like you haven't filled in the spaCy Contributor Agreement (SCA) yet. The agrement ensures that we can use your contribution across the project. Once you've filled in the template, put it in the .github/contributors directory and add it to this pull request. If your pull request targets a branch that's not master, for example develop, make sure to submit the Contributor Agreement to the master branch. Thanks a lot!

If you've already included the Contributor Agreement in your pull request above, you can ignore this message.

@ines ines added bug Bugs and behaviour differing from documentation feat / vectors Feature: Word vectors and similarity feat / cli Feature: Command-line interface labels Dec 27, 2018
@ines
Copy link
Member

ines commented Dec 27, 2018

Thanks, this was quick! (And ignore the bot 😛)

Still need to check if it fixes spacy package

I think the package command generates the meta separately – so this might have to be adjusted in generate_meta here.

@ines ines added ⚠️ wip Work in progress 🌙 nightly Discussion and contributions related to nightly builds labels Dec 27, 2018
@ines ines removed the ⚠️ wip Work in progress label Dec 27, 2018
@ines
Copy link
Member

ines commented Dec 27, 2018

Looks good – merging! 👍

@ines ines merged commit 0546135 into explosion:develop Dec 27, 2018
@jarib
Copy link
Contributor Author

jarib commented Dec 27, 2018

I've added vectors.name to spacy package's meta generation and tested that it successfully packages a model with vectors.

A future improvement could be to reduce the duplication by making the model meta data into a class that adds some validation.

@rbhambriiit
Copy link

Just hit the issue in 2.0.18

One of the smallest pull request for sure :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs and behaviour differing from documentation feat / cli Feature: Command-line interface feat / vectors Feature: Word vectors and similarity 🌙 nightly Discussion and contributions related to nightly builds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants