-
Notifications
You must be signed in to change notification settings - Fork 35
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
add dynamic dependency in pyproject.toml #98
Conversation
@Tieqiong Noticed the same bug with I checked cookiecutter`s pyproject.toml that it doesn't have the dependency install section. |
Thanks @Tieqiong I will test |
Very slow internet here so not sure if I can test befoer I have to board plane. But in answer to the other questions, I agree that adding a |
OK, I tested on local and it worked. I will push out a release bump to pypi if I can (internet!) |
@Tieqiong @bobleesj please can you test by pip installing I couldn't build the pycifrw dependency on my windows computer. I get:
(not the full trace) which is pretty annoying. |
I guess this is because pycifrw only deploys their code to pypi, as we do for pdffit2, and so a pip install of it tries to build locally and I don't have the right compiler. |
It works for me. Mac M1 Testing the import
|
@sbillinge It'll work with the Microsoft c++ compiler installed. |
I don't think this is testing what we want. You have to do an install in a new clean empty environment. |
@sbillinge Okay. pip install test passed from a new env using the recent 3.2.1 version.
|
@sbillinge I think in fact we can't include so I guess instead of skipping This is specific to |
so is everything fixed if we just remove python from build.txt? This seems like a good solution? |
as far as I know yes I think this will get rid of the error. However I'm not exactly sure if we want to remove it because I have no idea why it was included in the first place... |
Yah, if we don't understand what is going on we could just be introducing more problems |
@sbillinge So I searched around online and asked Andrew, it seems like there's nothing special with this |
What do you mean by "nothing special"? |
I didn't find any documentation that have it as convention, and according to Andrew:
So by "nothing special" I mean there's no hidden mechanism that require it to be there |
I guess a better way to go is to figure out how to build wheel so that build.txt would not be needed for pip install for sure. |
Do you mean that it doesn't have to be there? |
@Tieqiong please can you propose a solution for what you want the yml to look like? We will work on building the wheels but we also want the source bundle to be right either way. |
Yes it doesn't have to be there, but whether it should be there depends on how we are using the file (I think we are talking about build.txt not yml file right?). If we use the file as a source of information during pip installation, then we need to remove python because it'll cause error for pip. |
No human will read it, so let's get rid of the python in there. We want pip to be able to build from sources even if we distribute wheels |
closes #97
@sbillinge Please check and better test it on your setup.
We probably want this in the cookiecutter too, maybe also change the worked packages.
Another thing that might worth notice is do we want to include
requirements/build.txt
? This is specific todiffpy.pdffit2
'sgsl
dependency. Howeversetuptool
andpython
are included, which in theory should already be present if a user is installing this package to a conda environment with python installed.