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

Embracing Poetry: includes version + deps bump #779

Merged
merged 9 commits into from
Jul 15, 2022
Merged

Embracing Poetry: includes version + deps bump #779

merged 9 commits into from
Jul 15, 2022

Conversation

JackUrb
Copy link
Contributor

@JackUrb JackUrb commented May 23, 2022

Overview

Preparing for the release of 1.0.2, this PR moves us to fully adopt poetry as our python package manager. This is handled by moving all of the details that used to be in setup.py, requirements.txt, and MANIFEST.in into the pyproject.toml file. We're adapting this as we needed editable installations with pip install -e . as described in python-poetry/poetry#34, and following python-poetry/poetry-core#182 we could actually have it. A year late, but eventually is better than never!

After this it should be easier to modify and manage packages, and to deal with ParlAI dependencies (perhaps we can push them to adopt as well?).

Should resolve #818 and #768.

Note - some of the pyproject.toml file is still commented out. This is because there's some nicer syntax coming out in poetry 1.2 that I'd like to adopt, but I can't get tests to work using it just yet so I've left it commented out.

@JackUrb JackUrb requested a review from pringshia May 23, 2022 21:18
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 23, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 23, 2022

Codecov Report

Merging #779 (5bf5ae1) into main (dc8f72d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #779   +/-   ##
=======================================
  Coverage   64.57%   64.57%           
=======================================
  Files         107      107           
  Lines        9259     9259           
=======================================
  Hits         5979     5979           
  Misses       3280     3280           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc8f72d...5bf5ae1. Read the comment docs.

@JackUrb JackUrb changed the title Version bump + deps Embracing Poetry: includes version + deps bump Jul 14, 2022
@pringshia
Copy link
Contributor

Seems like there are a bunch of test failures - possible that the dependencies in pyproject.toml aren't up to date?

@pringshia
Copy link
Contributor

Will this mean pip install -e won't work anymore btw? In that case we should update our Github Actions tests and Dockerfiles to use poetry install instead of pip install -e.

https://github.com/facebookresearch/Mephisto/search?q=pip+install

@JackUrb
Copy link
Contributor Author

JackUrb commented Jul 14, 2022

Debugging the tests, but pip install -e . still works perfectly :)

@JackUrb
Copy link
Contributor Author

JackUrb commented Jul 14, 2022

Alright solved the issue by commenting out the fancy poetry 1.2 syntax, will return to that when the release is finally cut.

@JackUrb JackUrb mentioned this pull request Jul 15, 2022
2 tasks
@JackUrb JackUrb merged commit 3060246 into main Jul 15, 2022
@JackUrb JackUrb deleted the version-bump branch July 15, 2022 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors when installing new dependencies
4 participants