Skip to content

DOC Replace Canopy installers by Enthought Deployment Manager#17336

Merged
rth merged 3 commits intoscikit-learn:masterfrom
alfaro96:replace_canopy_by_edm
Jun 3, 2020
Merged

DOC Replace Canopy installers by Enthought Deployment Manager#17336
rth merged 3 commits intoscikit-learn:masterfrom
alfaro96:replace_canopy_by_edm

Conversation

@alfaro96
Copy link
Copy Markdown
Member

Reference Issues/PRs

Fixes #17311.

What does this implement/fix? Explain your changes.

This PR replaces the end-of-life Canopy installers by the new Enthought Deployment Manager in the installation instructions.

CC @cmarmo.

Copy link
Copy Markdown
Contributor

@cmarmo cmarmo left a comment

Choose a reason for hiding this comment

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

Thanks @alfaro96! LGTM for me. Perhaps you could just check and let us know how recent is the scikit-learn version in edm? Thanks!

@alfaro96
Copy link
Copy Markdown
Member Author

alfaro96 commented May 25, 2020

Thanks @alfaro96! LGTM for me. Perhaps you could just check and let us know how recent is the scikit-learn version in edm? Thanks!

For the latest EDM release (2.5.0):

python -c "import sklearn; print(sklearn.__version__)"

outputs:

0.21.3

That was released July 30, 2019 (almost ten months ago). IMO the scikit-learn version is quite old.

WDYT @cmarmo?

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented May 25, 2020

That was released July 30, 2019 (almost ten months ago). IMO the scikit-learn version is quite old.

WDYT @cmarmo?

Maybe you can skip the "recent" thing then... :)
@rth, @thomasjpfan WDYT? Thanks!

doc/install.rst Outdated

Canopy and Anaconda for all supported platforms
-----------------------------------------------
Enthought Deployment Manager and Anaconda for all supported platforms
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have a bias:

Suggested change
Enthought Deployment Manager and Anaconda for all supported platforms
Anaconda and Enthought Deployment Manager for all supported platforms

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

LGTM. In fact, I prefer this approach.

doc/install.rst Outdated
Comment on lines +227 to +228
`Enthought Deployment Manager <https://assets.enthought.com/downloads/>`_
and `Anaconda <https://www.anaconda.com/download>`_ both ship a recent
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The bias continues. Removing the word "recent" more or less deliveries the same message.

Suggested change
`Enthought Deployment Manager <https://assets.enthought.com/downloads/>`_
and `Anaconda <https://www.anaconda.com/download>`_ both ship a recent
`Anaconda <https://www.anaconda.com/download>`_ and
`Enthought Deployment Manager <https://assets.enthought.com/downloads/>`_ both ship a

@alfaro96 alfaro96 requested a review from thomasjpfan May 25, 2020 16:17
doc/install.rst Outdated
<https://www.enthought.com/products/canopy>`_ and `Anaconda
<https://www.anaconda.com/download>`_ both ship a recent
`Anaconda <https://www.anaconda.com/download>`_ and
`Enthought Deployment Manager <https://assets.enthought.com/downloads/>`_ both ship a recent
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the enthought is not "recent"

both ship with scikit-learn in addition

And wrap this so there is <80 characters per line.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@thomasjpfan Thanks for the comments!

@alfaro96 alfaro96 requested a review from thomasjpfan May 25, 2020 17:39
Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you @alfaro96 !

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented Jun 3, 2020

@NicolasHug , I think this PR is ready for merge, WDYT? Thanks!

Copy link
Copy Markdown
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Thanks!

@rth rth merged commit d9d5fb4 into scikit-learn:master Jun 3, 2020
@alfaro96 alfaro96 deleted the replace_canopy_by_edm branch June 24, 2020 11:53
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Broken link to Canopy installers

4 participants