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.

Sign up for GitHub

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

Jump to bottom

Adding torchcam from PyPI #17307

Merged
merged 7 commits into from
Dec 23, 2021
Merged

Adding torchcam from PyPI #17307

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6265f33
Adding `torchcam` from PyPI
sugatoray Dec 21, 2021
e676df4
reworded torch as pytorch
sugatoray Dec 21, 2021
f6e07e8
updated script section
sugatoray Dec 21, 2021
64e3539
removed two test lines from the script
sugatoray Dec 21, 2021
df31ca0
minor changes
sugatoray Dec 21, 2021
ee97be7
updated recipe
sugatoray Dec 23, 2021
a8cb6d1
updated recipe: include review-suggestions
sugatoray Dec 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 58 additions & 0 deletions recipes/torchcam/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
{% set name = "torchcam" %}
{% set version = "0.3.1" %}


package:
name: {{ name|lower }}
version: {{ version }}

source:
url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/torchcam-{{ version }}.tar.gz
sha256: 400d451cbbafe0dbddbf3a22feab9e23d1317e90cf6eae16bca96e0e2ff4f076

build:
number: 0
noarch: python
script:
## Ensure exclusion of tests folder
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@sugatoray sugatoray Dec 21, 2021 •
edited

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@sugatoray sugatoray Dec 21, 2021 •
edited

Choose a reason for hiding this comment

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

I will clean up the recipe afterwards (and remove anything extra). Including the conditionals are temporary measures only (until fixed upstream).

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right about that. Just removed the lines for "copying/creating requirements.txt" .

{% set testsfolder = "test" %}
- 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
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@sugatoray sugatoray Dec 23, 2021 •
edited

Choose a reason for hiding this comment

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

Done. Thank you.

Copy link
Member

Choose a reason for hiding this comment

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

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.

- {{ PYTHON }} -m pip install . -vv

requirements:
host:
- pip
- python >=3.6
run:
- python >=3.6
- matplotlib-base >=3.0.0
- numpy >=1.14.0
- pillow >=8.3.2
- pytorch >=1.5.1

test:
imports:
- torchcam
- torchcam.methods
commands:
- pip check
requires:
- pip

about:
home: https://github.com/frgfm/torch-cam
summary: Class activation maps for your PyTorch CNN models
license: Apache-2.0
license_file: LICENSE
description: |
Simple way to leverage the class-specific activation of
convolutional layers in PyTorch.
doc_url: https://frgfm.github.io/torch-cam
dev_url: https://github.com/frgfm/torch-cam

extra:
recipe-maintainers:
- sugatoray