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

Link to conda-forge in Readme #441

Merged
merged 6 commits into from Jun 25, 2019

Conversation

Projects
None yet
5 participants
@JanakiSheth
Copy link
Contributor

commented Feb 23, 2019

I found it useful to install skorch using conda-forge, and someone else might find it useful too.

@ottonemo

This comment has been minimized.

Copy link
Collaborator

commented Feb 25, 2019

Thank you for the pull request, this looks fine in general!

I'm not sure about linking to a 3rd party resource, though. I appreciate when people take the source and distribute skorch packages over some platform like conda-forge but I'm hesitant linking to it in the README since none of the skorch maintainers control the conda-forge repository. So in the worst case the conda-forge repository gets compromised without our knowledge and we encourage users to use the compromised package.

But maybe that's just paranoia. @BenjaminBossan what do you think?

@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator

commented Feb 25, 2019

In theory, it's possible that pytorch gets compromised and we encourage users to install it, so there is no absolute safety. But it would probably not hurt to mention that we are not responsible for the forge repo, since that could be something that users might assume when reading about it in the README.md.

@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator

commented Mar 2, 2019

@JanakiSheth Would you mind adding a note that the conda channel is not managed the skorch maintainers?

@hmaarrfk

This comment has been minimized.

Copy link

commented Mar 4, 2019

Hey, conda-forge contributor here. You can probably just submit the PR (using the github editor) to add yourselves to the bottom of the maintainers list explaining that you are a core dev of skorch. That way you would retain some control over the distribution.

https://github.com/conda-forge/skorch-feedstock/blob/master/recipe/meta.yaml#L47

Seeing as this seems like a pure python package, it should just be OK and minimal maintenance effort as bots do most of the work for pypi/github release packages.

@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator

commented Mar 5, 2019

@hmaarrfk Thx for the hint. I'm not quite sure how this works. Once you are on the maintainer list, you are able to directly push to the repo?

@hmaarrfk

This comment has been minimized.

Copy link

commented Mar 5, 2019

@benjamin-work yeah, conda-forge is basically a giant automation machine. They ask that you not push directly to the repo since it triggers packages to be released. Instead, you work on your fork and make a PR. Once everything is passing on CIs (travis/appveyor/circleci/azure), you can merge it in, and the package gets released to Anaconda automatically.

I think it would be instructive to package an other small project of yours to get the hang of things:
https://github.com/conda-forge/staged-recipes
is the place to start.
Instructions for how to use it are at the bottom of that link.

@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

@ottonemo Let's make some progress. Would you be fine with the change?

@thomasjpfan
Copy link
Member

left a comment

LTGM

ottonemo added some commits Jun 19, 2019

Update README.rst
Some grammar fixes, additional installation instructions, updated PyTorch installation instructions.
@ottonemo

This comment has been minimized.

Copy link
Collaborator

commented Jun 19, 2019

@thomasjpfan @BenjaminBossan please review :)

@thomasjpfan
Copy link
Member

left a comment

This looks good for the README. There are a couple of other places we need to update to skorch-dev.

@@ -223,15 +235,15 @@ In general, this should work (assuming CUDA 9):
.. code:: bash
# using conda:
conda install pytorch -c pytorch
conda install pytorch cudatoolkit=9.0 -c pytorch

This comment has been minimized.

Copy link
@thomasjpfan

thomasjpfan Jun 19, 2019

Member

Do we want users on 10.0?

This comment has been minimized.

Copy link
@ottonemo

ottonemo Jun 25, 2019

Collaborator

PyTorch uses 9 as the default, so we'll stick with 9 for now, I guess.

README.rst Outdated
@@ -12,7 +12,7 @@ A scikit-learn compatible neural network library that wraps PyTorch.
:scale: 100%
:target: https://travis-ci.org/dnouri/skorch?branch=master

This comment has been minimized.

Copy link
@thomasjpfan

thomasjpfan Jun 19, 2019

Member
.. image:: https://travis-ci.com/skorch-dev/skorch.svg?branch=master
    :target: https://travis-ci.com/skorch-dev/skorch

ottonemo added some commits Jun 25, 2019

Update README.rst
Update travis batch
Update README.rst
Update travis badge target

@ottonemo ottonemo merged commit afb6654 into skorch-dev:master Jun 25, 2019

0 of 2 checks passed

Travis CI - Pull Request Build Errored
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.