Adding torchcam from PyPI
#17307
Adding torchcam from PyPI
#17307
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
- inclusion of reqfile - exclision of tests folder - comments - echo out rendered recipe
|
PR is Ready. Request for review/merging. cc: @conda-forge/help-python @BastianZim @dopplershift @beckermr |
| - if [ ! -f {{ reqfile }} ] && [ -f {{ reqsource }} ]; then cp {{ reqsource }} {{ reqfile }}; fi # [not win] | ||
| #- copy {{ name }}.egg-info\requires.txt {{ reqfile }} # [win] | ||
| - copy {{ reqsource | replace('/', '\\') }} {{ reqfile }} # [win] | ||
| ## Ensure exclusion of tests folder |
There was a problem hiding this comment.
The best way to do this is by changing it upstream. Should be fine for now but would be good to have a PR with a permanent fix.
There was a problem hiding this comment.
Changing upstream could take longer and sometimes there's no response. I did this in another PR recently: #17275. The if conditionals will make sure of only creating the requirements.txt file when it is not present.
There was a problem hiding this comment.
I will clean up the recipe afterwards (and remove anything extra). Including the conditionals are temporary measures only (until fixed upstream).
There was a problem hiding this comment.
Changing upstream could take longer and sometimes there's no response.
Yes that's why I meant that it's fine for now but a PR would be good nonetheless.
The if conditionals will make sure of only creating the requirements.txt file when it is not present.
It seems like it's currently available so it wouldn't be needed?
There was a problem hiding this comment.
Yes, you are right about that. Just removed the lines for "copying/creating requirements.txt" .
Why is windows build failing?PyTorch is a dependency of |
- removed redundant scripts
| - echo -e "\n>>> ENSURE EXCLUSION OF FOLDER '{{ testsfolder }}' <<<\n\n" | ||
| - rm -rf {{ testsfolder }} # [not win] | ||
| - rmdir /s /q "{{ testsfolder | replace('/', '\\') }}" # [win] | ||
| ## Install package {{ name }} from PyPI |
There was a problem hiding this comment.
You can reduce these 6 lines to just a rm -rf test. No need for the comments b/c the commands are quite descriptive and no need to do that on Windows b/c the recipe is noarch.
There was a problem hiding this comment.
Done. Thank you.
There was a problem hiding this comment.
The comments are not really needed and you can remove the selector too. Also, the jinja won't work on a comment. Merging this as these are all minor comments.
|
@ocefpaf Did what you suggested. Could this be merged now? |
|
Thank you for your review and help in merging the PR. |
Checklist
NAurl) rather than a repo (e.g.git_url) is used in your recipe (see here for more details).