feat(ci): drop setup.py from publishing CI - #737
Conversation
|
thanks @SauravMaheshkar . This has the big disadvantage that the versions are now duplicated in both optax/init.py and in the pyproject.toml . How about getting the version with |
Yes that would work, however that would require for us to do a editable install in that step i.e. a Although with some cache I'm sure even that could be sped up. Would you recommend I proceed with the proposed fix ? |
|
do we really need to install optax for that? I think as long as we run it from the optax root directory it should work |
|
We would run into import error's right ? Following is my terminal output for running ❯ python -c "import optax; print(optax.__version__)"
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/Users/saurav/github/SauravMaheshkar/optax/optax/__init__.py", line 17, in <module>
from optax import contrib
File "/Users/saurav/github/SauravMaheshkar/optax/optax/contrib/__init__.py", line 17, in <module>
from optax.contrib.cocob import cocob
File "/Users/saurav/github/SauravMaheshkar/optax/optax/contrib/cocob.py", line 23, in <module>
import jax.numpy as jnp
ModuleNotFoundError: No module named 'jax' |
|
works for me, it seems you're just missing some dependencies |
|
The python environment in the Github Actions workflow won't have Sorry if I'm missing something 😅 |
|
yes, you're absolutely right. I was thinking we could run |
|
which I see is what you proposed a few comments earlier 🙃 . Anyway, we seem to be on the same page 😄 |
|
Sounds good, will revert the change to |
|
My point before is that we don't really need to install optax, just the dependencies. Not sure if there's an easy way to do that, and TBH doesn't change much |
|
I thought |
Thanks! Let me know when I should take another look at it |
|
CI looks green. Should be good now @fabianp |
|
Thanks! Looks good to me |
Context
This PR aims to continue the work done in #513 to remove
setup.pyand migrate to a purepyproject.tomlbased build. While the PR was merged in (#513 (comment)) the older legacy files such assetup.pyandMANIFEST.instill exist.The comment to delete such legacy files was raised again #513 (comment) and thus the issue to drop
setup.pyfrom CI raised in #513 (comment).Changes made my this PR can be summarised as follows:
pyproject.toml.pyproject.toml.Request for Review: @fabianp