Skip to content

fix issue 436#47

Open
mszhanyi wants to merge 3 commits into
fastai:masterfrom
mszhanyi:master
Open

fix issue 436#47
mszhanyi wants to merge 3 commits into
fastai:masterfrom
mszhanyi:master

Conversation

@mszhanyi

@mszhanyi mszhanyi commented Apr 13, 2021

Copy link
Copy Markdown

Fix AnswerDotAI/nbdev#436
Depends on AnswerDotAI/fastcore#305

nbdev isn't a pure python package because it generates many cli command.
So, in meta.yml, we shouldn't use noarch:python but win-64:python.

After testing, script in build section should be python setup.py install --single-version-externally-managed --record=record.txt.
(ref: conda/conda-build#3200 (comment))

For windows, we need to create 3 conda package (python3,6, 3,7, 3.8), so I added conda_build_config.yaml and update anaconda_update to support batch uploading.

It changes fastrelease a lot, If there's better methods please let me know.

@jph00 @hamelsmu
The tests failed because there's no token. I think the token should be set by CI. Am I right?
I think I have no permission to upload nbdev package in Anaconda, so I only tested locally not test it in cloud.

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jph00

jph00 commented Apr 29, 2021

Copy link
Copy Markdown
Member

@mszhanyi this PR removes some functionality we rely on, that ensures we get the correct version. Is that functionality difficult to support on Windows? What are the issues you faced?

Regarding the failing CI, that seems to be an issue with the project settings. I'll check that now.

@mszhanyi

mszhanyi commented Apr 29, 2021

Copy link
Copy Markdown
Author

@mszhanyi this PR removes some functionality we rely on, that ensures we get the correct version. Is that functionality difficult to support on Windows? What are the issues you faced?

Regarding the failing CI, that seems to be an issue with the project settings. I'll check that now.

@jph00 , Do you mean the change in conda_output_path?
It is because there're 3 bz packages(py36, py37, py38) for nbdev on Windows, so I plan to upload them all together.
It has to create different package for different python if we choose win-64:python in build section

@mszhanyi

Copy link
Copy Markdown
Author

Just a reminder, please don't forget to add --pure_pytorch false when you verify it with nbdev on Windows.
fastrelease_conda_package --pure_python false.

@jph00

jph00 commented Apr 29, 2021

Copy link
Copy Markdown
Member

What I'm referring to is removing version param from anaconda_upload(), which is a breaking change to the public API.

Would you be able to put that back and ensure it works as before? To upload all the packages, you could either just loop thru them, or add some kind of wildcard param or similar to anaconda_upload().

@mszhanyi

Copy link
Copy Markdown
Author

It's a mistake, I remember I took it as python version at some point. It's recovered now.
I don't try to upload it. I think I have no permission and I'm afraid I couldn't revert it once there's error after uploading.

The results before uploading is below.
Linux:
image

Windows:
image

Does the return value matter? I don't set the return value because I need upload multiple times on Windows.

@mszhanyi

mszhanyi commented May 5, 2021

Copy link
Copy Markdown
Author

@jph00 is there more comments?

@jph00

jph00 commented May 5, 2021

Copy link
Copy Markdown
Member

@mszhanyi I'm still thinking about it...

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.

nbdev cli tools not installing on windows with conda

2 participants