dvc: support Python3.8 #3463

skshetry · 2020-03-10T03:48:13Z

PyInstaller does not support Python3.8 yet: pyinstaller/pyinstaller#4311

❗ Have you followed the guidelines in the Contributing to DVC list?
📖 Check this box if this PR does not require documentation updates, or if it does and you have created a separate PR in dvc.org with such updates (or at least opened an issue about it in that repo). Please link below to your PR (or issue) in the dvc.org repo.
❌ Have you checked DeepSource, CodeClimate, and other sanity checks below? We consider their findings recommendatory and don't expect everything to be addressed. Please review them carefully and fix those that actually improve code or fix bugs.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

TODO

Cherry-pick 625de04 to see if this makes 3.8 tests pass.
~~Try to update pyarrow to 0.16.0 for other versions of Python and make them pass.~~
Not possible due to https://issues.apache.org/jira/browse/ARROW-7852.
Enable 3.8 on Windows ~~and Mac~~.
Run formattings on 3.8. (Not necessary).
Build wheels for 3.8 as well, and advertise on classifiers in setup.py.
Removing cherrypick after tests: remove use of mocker as context-manager #3480 is merged. Code cleanups.

Future discussions

We should try to be ahead of the PyArrow regarding releases. So, we should at-least be able to release non-hdfs packages before/on release of new python versions. We can run tests on 3.9-dev without PyArrow.

Do we need to support Python3.5? The number of downloads are already low (and, numpy/conda does not support this version at all).

efiop · 2020-03-10T09:17:25Z

@skshetry Is this related to pyarrow issue from discord? If so, our wheels have nothing to do with that, as we don't package pyarrow wheels.

skshetry · 2020-03-10T09:22:14Z

pyarrow now has wheels for Python3.8. See https://pypi.org/project/pyarrow/#files

codecov · 2020-03-10T09:41:23Z

Codecov Report

Merging #3463 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3463      +/-   ##
==========================================
+ Coverage   93.04%   93.08%   +0.04%     
==========================================
  Files         141      141              
  Lines        8598     8768     +170     
==========================================
+ Hits         8000     8162     +162     
- Misses        598      606       +8

Impacted Files	Coverage Δ
dvc/system.py	`89.34% <100%> (+1.27%)`	⬆️
dvc/compat.py	`16.66% <0%> (-55.56%)`	⬇️
dvc/remote/slow_link_detection.py	`100% <0%> (ø)`	⬆️
dvc/command/data_sync.py	`100% <0%> (ø)`	⬆️
dvc/repo/destroy.py	`100% <0%> (ø)`	⬆️
dvc/rwlock.py	`100% <0%> (ø)`	⬆️
dvc/dependency/repo.py	`100% <0%> (ø)`	⬆️
dvc/repo/move.py	`100% <0%> (ø)`	⬆️
dvc/scm/tree.py	`100% <0%> (ø)`	⬆️
dvc/progress.py	`98.18% <0%> (+0.03%)`	⬆️
... and 32 more

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 d2734df...10078db. Read the comment docs.

efiop · 2020-03-10T09:46:57Z

@skshetry Ok, so could you explain what is going on here and what is the purpose?

skshetry · 2020-03-10T09:56:04Z

@skshetry Ok, so could you explain what is going on here and what is the purpose?

Umm, supporting Python3.8? Not sure of the question. And, trying to run tests on it.

efiop · 2020-03-10T13:44:17Z

Related conda-forge/dvc-feedstock#93

casperdcl · 2020-03-12T23:39:28Z

also fixes #3033; related to #2912

setup.py

.travis.yml

ci: set ARROW_LIBHDFS_DIR to load native library deps: pin pyarrow to 0.15.1 for py<=3.7 pin deps for python<3.8 pin deps for python==3.8 as well add windows Py38 on travis do not use speedcopy for Python3.8 and Windows

skshetry · 2020-03-16T06:21:21Z

tests/func/test_checkout.py


        self.assertTrue(System.is_symlink(self.FOO))
-        old_foo_link = os.readlink(self.FOO)
+        old_foo_link = os.path.realpath(self.FOO)


Using realpath due to change in how readlink works in Python3.8 and Windows: https://docs.python.org/3.8/library/os.html#os.readlink)

skshetry · 2020-03-16T06:23:55Z

setup.py

    "networkx>=2.1,<2.4",
    "pydot>=1.2.4",
-    "speedcopy>=2.0.1; sys_platform == 'win32'",
+    "speedcopy>=2.0.1; python_version < '3.8' and sys_platform == 'win32'",


A lot of dependencies don't have classifiers for Python3.8, but seems to have worked.

pmrowla

Other than the pyinstaller 3.8 support question, everything looks fine to me

efiop · 2020-03-16T14:52:41Z

scripts/ci/install_hadoop.sh

+# regression on PyArrow==0.16.0: https://issues.apache.org/jira/browse/ARROW-7841
+# it tries to retrieve native library from $HADOOP_HOME directory, instead of
+# `$HADOOP_HOME/lib/native`.
+# force it to search for `libhdfs.so` inside `$HADOOP_HOME/lib/native`


This regression is fixed in 0.17.0, right? Let's add a note about it as well, so we know that we can drop this line later.

fixed, will squash merge (I guess that's what we do now?) Also I presume @skshetry should PR from forks rather than base repo branches?

@casperdcl, I wanted to run all of the tests on the upstream branch. PR from forks don't run all tests.

efiop · 2020-03-16T14:58:03Z

@skshetry Amazing job! 🔥

We should try to be ahead of the PyArrow regarding releases. So, we should at-least be able to release non-hdfs packages before/on release of new python versions. We can run tests on 3.9-dev without PyArrow.

Really good point. Have you tried running dvc on 3.9? Is it only pyarrow that is causing us issues?

Do we need to support Python3.5? The number of downloads are already low (and, numpy/conda does not support this version at all).

Good point. We've considered dropping support for it, but decided to just wait for EOL that will be in December.

.travis.yml

setup.py

efiop

Please address the ordering issue and feel free to merge if there are no other reviews/issues to address 👍 Thank you!

casperdcl · 2020-03-16T19:42:10Z

Hmm apparently the "merge after checks pass" button doesn't work as intended (so the branch was deleted before tests completed). 🤔 tests should be fine though, no code changes.

Post-merge test: https://travis-ci.com/github/iterative/dvc/builds/153556761 seems fine

skshetry · 2020-03-17T05:03:26Z

Many thanks @casperdcl on moving this ahead. 🙂

skshetry force-pushed the python3.8 branch from 6efa71f to 6258493 Compare March 10, 2020 09:26

skshetry commented Mar 15, 2020

View reviewed changes

setup.py Outdated Show resolved Hide resolved

skshetry commented Mar 15, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

skshetry commented Mar 16, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

py38: make wheels

172e61e

ci: set ARROW_LIBHDFS_DIR to load native library deps: pin pyarrow to 0.15.1 for py<=3.7 pin deps for python<3.8 pin deps for python==3.8 as well add windows Py38 on travis do not use speedcopy for Python3.8 and Windows

skshetry force-pushed the python3.8 branch from bb29e42 to 172e61e Compare March 16, 2020 03:16

use realpath to resolve symbolic links

3374ecc

skshetry force-pushed the python3.8 branch from 798e423 to 3374ecc Compare March 16, 2020 04:49

do not build pkgs for Windows and Linux on Python3.8

8255c47

skshetry commented Mar 16, 2020

View reviewed changes

skshetry requested review from casperdcl, efiop and pared March 16, 2020 06:33

skshetry self-assigned this Mar 16, 2020

skshetry marked this pull request as ready for review March 16, 2020 06:34

skshetry added the build label Mar 16, 2020

skshetry changed the title ~~[WIP] py38: make wheels~~ py38: support Python3.8 Mar 16, 2020

skshetry changed the title ~~py38: support Python3.8~~ dvc: support Python3.8 Mar 16, 2020

skshetry requested review from casperdcl and pmrowla and removed request for casperdcl March 16, 2020 07:02

pmrowla reviewed Mar 16, 2020

View reviewed changes

efiop reviewed Mar 16, 2020

View reviewed changes

casperdcl suggested changes Mar 16, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

.travis.yml Show resolved Hide resolved

setup.py Show resolved Hide resolved

setup.py Show resolved Hide resolved

efiop approved these changes Mar 16, 2020

View reviewed changes

casperdcl added 2 commits March 16, 2020 19:09

reorder

1a8efcf

pyarrow: document regression fix

10078db

casperdcl merged commit 109ded4 into master Mar 16, 2020

casperdcl deleted the python3.8 branch March 16, 2020 19:18

dvc: support Python3.8 #3463

dvc: support Python3.8 #3463

Uh oh!

Conversation

skshetry commented Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Future discussions

Uh oh!

efiop commented Mar 10, 2020

Uh oh!

skshetry commented Mar 10, 2020

Uh oh!

codecov bot commented Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

efiop commented Mar 10, 2020

Uh oh!

skshetry commented Mar 10, 2020

Uh oh!

efiop commented Mar 10, 2020

Uh oh!

casperdcl commented Mar 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skshetry Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skshetry Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

pmrowla left a comment

Choose a reason for hiding this comment

Uh oh!

efiop Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

casperdcl Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

skshetry Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efiop commented Mar 16, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

efiop left a comment

Choose a reason for hiding this comment

Uh oh!

casperdcl commented Mar 16, 2020

Uh oh!

skshetry commented Mar 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

skshetry commented Mar 10, 2020 •

edited

Loading

codecov bot commented Mar 10, 2020 •

edited

Loading

skshetry Mar 16, 2020 •

edited

Loading

skshetry Mar 17, 2020 •

edited

Loading