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

Add install instruction for conda #188

Merged
merged 5 commits into from
Dec 6, 2023
Merged

Conversation

sugatoray
Copy link
Contributor

  • added install instruction
  • restructured installation and getting started section
  • added conda-forge badge ti easily keep track of PyPI vs. Conda-Forge package versions.

Closes #184.

- added install instruction
- restructured installation and getting started section
- added conda-forge badge ti easily keep track of PyPI vs. Conda-Forge package versions.
@sugatoray
Copy link
Contributor Author

cc: @goodmami

Copy link
Owner

@goodmami goodmami left a comment

Choose a reason for hiding this comment

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

Thanks for this! I have a few requested changes below. Also I would like to see a section added to the documentation beyond the README; namely in docs/setup.rst.

README.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 44 to 63

> 🔔 Tip: **Speeding up Conda Installations**
>
> To speedup conda installations, alternatively, consider installing `mamba`
> from conda-forge channel and then replace all `conda` commands with `mamba`
> as a drop-in replacement.
>
> ```sh
> conda install -c conda-forge mamba
> mamba install -c conda-forge wn
> ```

For more helpful tips on conda, refer to this handy [Conda Cheatsheet ↗️][#conda-cheatsheet]. :fire:

> ℹ️ The conda-forge recipe for this python library is maintained at the
> following conda-forge **feedstock** repository.
>
> - **Feedstock**: <https://github.com/conda-forge/wn-feedstock>

[#conda-cheatsheet]: https://conda.io/projects/conda/en/latest/_downloads/a35958a2a7fa1e927e7dfb61ebcd69a9/conda-4.14.pdf
Copy link
Owner

Choose a reason for hiding this comment

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

This is helpful information, but it's also not directly relevant to Wn and doesn't belong here in the README. I would like new users to see the install commands and the "Getting Started" example in the same screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I see your point. This may serve as a bit of distraction. Since the information is useful, let me put it under an expandable <details> block.

If someone wants to see the details, can click on it and see the additional useful information.

Copy link
Owner

Choose a reason for hiding this comment

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

Useful or not, it's not relevant. By this I mean that Wn's README is not the place to advertise alternative utilities or to teach people how to use conda. People can look that up, and also I don't want to have to update the README when the utilities or instructions change.

The link to the feedstock is relevant, but it is already at the top as a badge.

So while I appreciate your effort to provide helpful instructions here, I think the most I'd accept is linking the word conda to https://conda.io:

Or, install using [**conda**](https://conda.io), from conda-forge channel:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The link to the feedstock is relevant, but it is already at the top as a badge.

The badge point to the package on anaconda. Not the feedstock. This is common design for all packages. I would suggest you keep this information.

So while I appreciate your effort to provide helpful instructions here, I think the most I'd accept is linking the word conda to conda.io

The documentation of conda is enormous. And to my experience, a lot of folks do not know about these tricks. I understand your view on "non-advertising" and "keeping it short". I will drop the cheatsheet and the mamba install options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the README.

README.md Outdated Show resolved Hide resolved
@sugatoray
Copy link
Contributor Author

sugatoray commented May 14, 2023

Thanks for this! I have a few requested changes below. Also I would like to see a section added to the documentation beyond the README; namely in docs/setup.rst.

Let's get the README.md finalized first. I will go ahead and update the .rst file afterwards.

Copy link
Owner

@goodmami goodmami left a comment

Choose a reason for hiding this comment

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

I had to resolve a merge conflict, but I'll approve the rest so I can merge it. Thanks!

(edit: and sorry for the delay!)

@goodmami goodmami merged commit da42dd8 into goodmami:main Dec 6, 2023
12 checks passed
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.

Add a conda install option for wn on conda-forge channel
2 participants