Skip to content

Conversation

@adam2392
Copy link
Contributor

@adam2392 adam2392 commented Nov 8, 2023

Closes: #138

@adam2392
Copy link
Contributor Author

adam2392 commented Nov 8, 2023

Just want to clarify that this is moving in the right direction?

Copy link
Member

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

I've left some comments, but this is pretty much the gist of it, yes!

spin/cmds/pip.py Outdated
pip arguments are passed through in the same manner as you would pip e.g.:
spin install -- -r requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Would this example make sense? I'd read it as:

pip install . --editable --no-build-isolation -r requirements.txt

which doesn't make so much sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay, I clarified and edited slightly. With the new logic of the install function, hopefully this is better?

Copy link
Member

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

Looking good; you should also add this to the CI test.sh to ensure that it does what you expect it to.

spin/cmds/pip.py Outdated
pip arguments are passed through in the same manner as you would pip e.g.:
spin install -- -r requirements.txt --editable false
Copy link
Member

Choose a reason for hiding this comment

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

My point was that you probably don't want to do this. You want something like: spin install -- --no-clean or another option that makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see. sure that makes sense. I edited the example

spin/cmds/pip.py Outdated
Comment on lines 28 to 29
if editable:
pip_args += ["--no-build-isolation", "--editable"]
Copy link
Member

Choose a reason for hiding this comment

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

Maybe --no-build-isolation should be used in both cases? I.e.,pip_cmd = ["pip", "install", ".", "--no-build-isolation"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure fair.

@stefanv
Copy link
Member

stefanv commented Nov 9, 2023

And to pyproject.toml of the example project.

@adam2392
Copy link
Contributor Author

adam2392 commented Nov 9, 2023

Few questions:

  1. Should I add a changelog (edited by @jarrodmillman ) The changelog is generated automatically from PR titles, so no need to do anything for this.
  2. Is there a documented way to test the example package locally?

@jarrodmillman jarrodmillman added the type: Enhancement New feature or request label Nov 9, 2023
@jarrodmillman jarrodmillman changed the title [ENH] Adding pip install with editable mode Add pip install with editable mode Nov 9, 2023
@stefanv stefanv merged commit 018db2e into scientific-python:main Nov 13, 2023
@stefanv
Copy link
Member

stefanv commented Nov 13, 2023

Thanks @adam2392!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Is it possible to add a command for editable installs?

3 participants