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

fix to conda install instructions in readme #610

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

Tom-A-Lynch
Copy link
Contributor

The current instructions for installing jupyter-ai with conda/mamba are wrong.

This is a simple change to the readme to bring it inline with instructions found on conda-forge

Copy link

welcome bot commented Jan 31, 2024

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@Tom-A-Lynch Tom-A-Lynch marked this pull request as draft January 31, 2024 22:31
@krassowski
Copy link
Member

Can you define wrong? Did it throw an error when you run it?

Maybe it should be using dash rather than underscore. I am not very familiar with the new syntax that you propose, do you know which conda versions is it compatible with?

@JasonWeill
Copy link
Collaborator

I do see the channel::package syntax documented here for use in search: https://docs.conda.io/projects/conda/en/latest/user-guide/concepts/pkg-search.html

I haven't used this syntax for installing packages, though. conda install conda-forge::jupyter-ai does work, but it doesn't seem to be the only way to install our package.

@Tom-A-Lynch Tom-A-Lynch marked this pull request as ready for review January 31, 2024 22:52
@Tom-A-Lynch
Copy link
Contributor Author

Hi! So theres two changes here, one being the syntax and the other being the dash

when you install with the underscore it fails, it needs to be referenced as jupyter-ai from my experience

the second change was made to be more indicitive of how the jupyter stacks team formats their conda installs for standard jupyterlab images!

@Tom-A-Lynch
Copy link
Contributor Author

see this jupyter/docker-stacks Dockerfile where they install many packages at once on line 30.

https://github.com/jupyter/docker-stacks/blob/main/images/scipy-notebook/Dockerfile

@Tom-A-Lynch
Copy link
Contributor Author

Tom-A-Lynch commented Jan 31, 2024

Can you define wrong? Did it throw an error when you run it?

Maybe it should be using dash rather than underscore. I am not very familiar with the new syntax that you propose, do you know which conda versions is it compatible with?

I tried doing some research on this. I cant find exactly when this syntax was introduced, but its used commonly in the documentation now.

https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-channels.html

And yeah, the only thing explicitly wrong is the jupyter_ai instead of jupyter-ai.

I can change this PR to only introduce that change if you like.

this syntax change is just more friendly for when you're building jupyter-ai into a jupyterlab image to be served.

@JasonWeill
Copy link
Collaborator

I just ran conda install -c conda-forge jupyter_ai and I got an error. It seems like the hyphen is required in this case.

As for the syntax, it looks like both the old (-c conda-forge) and the new conda-forge::jupyter-ai syntaxes are supported by the install command. Even though the old syntax isn't apparently deprecated, your change does fix an incorrect command in the README, so it looks good to me. Thank you for your contribution!

@JasonWeill JasonWeill added the bug Something isn't working label Jan 31, 2024
@JasonWeill JasonWeill merged commit fb77735 into jupyterlab:main Jan 31, 2024
8 of 9 checks passed
Copy link

welcome bot commented Jan 31, 2024

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@JasonWeill
Copy link
Collaborator

@meeseeksdev please backport to 1.x

Copy link

lumberbot-app bot commented Jan 31, 2024

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 1.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 fb777350e843bc1aa35e484b5356a51c3bef9df2
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #610: fix to conda install instructions in readme'
  1. Push to a named branch:
git push YOURFORK 1.x:auto-backport-of-pr-610-on-1.x
  1. Create a PR against branch 1.x, I would have named this PR:

"Backport PR #610 on branch 1.x (fix to conda install instructions in readme)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

JasonWeill pushed a commit to JasonWeill/jupyter-ai that referenced this pull request Jan 31, 2024
JasonWeill added a commit that referenced this pull request Jan 31, 2024
* Base chat handler refactor for custom slash commands (#398)

* Adds attributes, starts adding to subclasses

* Consistent syntax

* Help for all handlers

* Fix slash ID error

* Iterate through entry points

* Fix typo in call to select()

* Moves config to magics, modifies extensions to attempt to load classes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Moves config to proper location, improves error logging

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* WIP: Updates per feedback, adds custom handler

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removes redundant code, style fixes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removes unnecessary custom message

* Instantiates class

* Validates slash ID

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Consistent arguments to chat handlers

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Refactors to avoid intentionally unused params

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Updates docs, removes custom handler from source and config

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Renames process_message to match base class

* Adds needed parameter that had been deleted

* Joins lines in contributor doc

* Removes natural language routing type, which is not yet used

* Update docs/source/developers/index.md

Co-authored-by: Piyush Jain <[email protected]>

* Update docs/source/developers/index.md

Co-authored-by: Piyush Jain <[email protected]>

* Update docs/source/developers/index.md

Co-authored-by: Piyush Jain <[email protected]>

* Revises per @3coins, avoids Latinism

* Removes Configurable, since we do not yet have configurable traits

* Uses Literal for validation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Piyush Jain <[email protected]>
(cherry picked from commit 64e6daf)

* Removes redundant pydantic import

* fix to conda install instructions in readme (#610)

(cherry picked from commit fb77735)

---------

Co-authored-by: Tom Lynch <[email protected]>
@krassowski
Copy link
Member

Even though the old syntax isn't apparently deprecated,

What I was hinting at is that there might be older versions which do not support the new syntax. It would be easier to say if conda documentation included "version added" comment; from the release notes I can only infer it was already present in January last year (2023) since these mention a documentation for this feature being added.

@JasonWeill
Copy link
Collaborator

Both the "old" and "new" syntax will be in the full docs once #611 is merged (thanks @krassowski for your feedback on that PR, too!). We can revise the README and docs if one syntax or the other gets deleted.

dbelgrod pushed a commit to dbelgrod/jupyter-ai that referenced this pull request Jun 10, 2024
Marchlak pushed a commit to Marchlak/jupyter-ai that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants