Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Update Python wheel with new myelin and task components #184

Merged
merged 1 commit into from
May 16, 2018

Conversation

ringgaard
Copy link
Contributor

I have updated the Python wheel to include the new myelin and task components. I have moved the myelin components from sling/nlp/parser/trainer/flow to python/myelin and removed the old flow.py module that is no longer needed after we removed the TF trainer.

I have updated the version number for the sling wheel to 2.0.0 to distinguish the caspar version from the sempar version.

@ringgaard ringgaard self-assigned this May 16, 2018
@ringgaard ringgaard requested a review from rahul1980 May 16, 2018 12:08
import nn as flownn
import flow as flow
import builder as builder
import sling.myelin.nn as flownn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this work without any other changes? e.g. do we also need to change python/init.py?
Note that the new path of nn.py is python/myelin/ and not sling/myelin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will work although I have not tested it (shame on me). When you run the parser trainer, we assume that you have pysling installed (or linked), so the Python package directory will contain a sling subdirectory that will contain myelin/nn.py.

The declarations in python/__init__.py are only needed to alias classes into the top sling namespace, e.g. the Document class is actual sling.nlp.document.Document, but we alias this so you can use this as sling.Document. We can consider doing this for some of the myelin classes but we are not relying on that at the moment.

@ringgaard ringgaard merged commit 8d3bb6e into google:caspar May 16, 2018
@ringgaard ringgaard deleted the wheel branch May 16, 2018 18:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants