-
Notifications
You must be signed in to change notification settings - Fork 254
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
Change dependencies for Python 3.10 support #446
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Couple of minor points about what versions we should specify but generally happy with the upgrade.
Note CI will still be running on 3.8 as Dockerfile not changed. But we probably want that so long as we want to maintain compatibility with 3.8, as Python is backwards compatible more likely to break on 3.8 and not 3.10 than vice-versa.
Codecov Report
@@ Coverage Diff @@
## master #446 +/- ##
==========================================
- Coverage 96.96% 96.56% -0.40%
==========================================
Files 79 79
Lines 6264 6263 -1
==========================================
- Hits 6074 6048 -26
- Misses 190 215 +25
Continue to review full report at Codecov.
|
Fixes #399 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Currently,
imitation
does not work properly on Python 3.10 due to its reliance on the PyPI version of Sacred (0.8.2). In particular, if you try to runpython -m imitation.scripts.train_rl
, you'll get errors like this:The
Mapping
type was removed from thecollections
library in Python 3.10. The GitHub version of Sacred was actually updated for Python 3.10 back in March (it's now on 0.8.3), but unfortunately the maintainer in charge of pushing new releases to PyPI seems to be MIA (see IDSIA/sacred#857), so it's not clear when the PyPI package will be updated. This PR fixes the issue by simply changing the Sacred dependency insetup.py
to the GitHub version.I also noticed that the tests currently require an out-of-date version of Ray Tune (~=0.8.5), but Ray just added support for Python 3.10 in its most recent release a few days ago (1.13.0). So I changed that dependency from
~=0.8.5
to>=0.8.5
to allow for downloading the most recent version for testing.