-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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.
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
Added recipe for crypten
#15720
Added recipe for crypten
#15720
Conversation
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/crypten:
|
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 ( |
Hi friend! We really, really, really appreciate that you have taken the time to make a PR on In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on Cheers and thank you for contributing to this community effort! |
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/crypten:
|
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 ( |
Hi @conda-forge/python, @conda-forge/staged-recipes, this is ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few cleanup points.
source: | ||
- url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz | ||
sha256: f9d76bc0ba1468cb77261b8d5486ee875266363310353a18aba948429aa24fda | ||
- url: https://raw.githubusercontent.com/facebookresearch/CrypTen/main/requirements.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need this file? It repeats the information in the meta.yaml
file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will make a PR once this recipe is ready, and then make the change appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this recipe is nearly ready once you remove the requirements.txt
source. You can then remove one level of nesting in your url list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but since I am not sure when the other PR would get merged, I was hoping to have the recipe available. I could then make for a stronger case for the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added the PR: facebookresearch/CrypTen#350
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to make the PR. that file is not necessary to be distributed. it is distributed as metadata along with the wheel or the conda package. Simply remove it from the recipe. You've copied the contents in the meta already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the commit right before that, 88f9835, did not run with the error that the requirements.txt
file is not present [ref]:
Traceback (most recent call last):
File "<string>", line 2, in <module>
File "<pip-setuptools-caller>", line 34, in <module>
File "/home/conda/staged-recipes/build_artifacts/crypten_1644693818666/work/setup.py", line 20, in <module>
with open("requirements.txt") as f:
FileNotFoundError: [Errno 2] No such file or directory: 'requirements.txt'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh that is too bad. Yeah. this seems OK. however you aren't taking a specific tag or commit. I suspect this will be problematic in the future. You should for yourself include a cross refernece to the issue / PR you raised upstream. But this looks good now.
Co-authored-by: Mark Harfouche <[email protected]>
Awesome, thanks @hmaarrfk |
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).