Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

add install instructions to README #32

Merged
merged 3 commits into from
Apr 29, 2020
Merged

add install instructions to README #32

merged 3 commits into from
Apr 29, 2020

Conversation

epwalsh
Copy link
Member

@epwalsh epwalsh commented Apr 29, 2020

This should clarify allenai/allennlp#4155

@epwalsh epwalsh requested review from dirkgr and nelson-liu April 29, 2020 17:03
### From source

If you intend to install the models package from source, then you probably also want to [install `allennlp` from source](https://github.com/allenai/allennlp#installing-from-source).
Once you have `allennlp` installed, run the following within the same Python environment:
Copy link
Contributor

Choose a reason for hiding this comment

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

what version of allennlp should you be using with the latest models version? is it expected that models is updated lockstep with allennlp-core (i.e., always make sure you have the latest master for both)?

Copy link
Member Author

Choose a reason for hiding this comment

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

They are developed and tested side-by-side, so they should be kept up-to-date with each other. If you look at the GitHub Actions workflow for allennlp-models, it's always tested against the master branch of allennlp. Similarly, allennlp is always tested against the master branch of allennlp-models.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's good to know, thanks! Do you think that information should be added to the PR? I guess i'm imagining a scenario where I have to go to a previous commit of allennlp-model or allennlp-core, and your information would be helpful in figuring out what corresponding other hash i need.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just added!

Copy link
Contributor

@nelson-liu nelson-liu left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to write this up, it's very helpful!

@epwalsh epwalsh merged commit 3a50436 into master Apr 29, 2020
@epwalsh epwalsh deleted the readme branch April 29, 2020 18:30
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