PKG Add msprime and deps including libgsl #2548
Conversation
benjeffery
force-pushed
the
add_msprime
branch
2 times, most recently
from
3f6597c
to
e905c29
Compare
benjeffery
changed the title
benjeffery
force-pushed
the
add_msprime
branch
11 times, most recently
from
d461471
to
72467f8
Compare
|
This PR is now ready for review/merge. Thanks again for the great docs and tooling, I was able to get |
There was a problem hiding this comment.
Thanks for doing this work @benjeffery ! A few comments otherwise LGTM.
packages/msprime/test_msprime.py
Outdated
| "tskit", | ||
| "demes", | ||
| "msprime", | ||
| "newick", |
There was a problem hiding this comment.
Normally you don't need to specify packages that are already dependencies of msprime. It's probably a good idea to omit those to check that dependencies are correctly installed.
packages/msprime/meta.yaml
Outdated
| sha256: e840bed2ba37ae572b234f0b55af006dc408881593f184304481ee6058ea6f30 | ||
| build: | ||
| script: | | ||
| pip install -t $HOSTSITEPACKAGES numpy |
There was a problem hiding this comment.
So numpy is a build dependency as far as I understand?
It should already be installed on when building this package. So either it should build without this line altogether, or if it doesn't work you might need to adjust cflags/ldfags to include the right folders see for example
pyodide/packages/scipy/meta.yaml
Line 42 in dc29ae6
There was a problem hiding this comment.
Due to the way msprime is currently built it's setup.py imports numpy. This is planned to be fixed soon, at which point I will remove this. I've opened tskit-dev/msprime#2055 to track.
There was a problem hiding this comment.
Right, so numpy is a build time dependency -- we have many such packages packaged (e.g. scipy, pandas), it's not an issue as far as Pyodide is concerned. My previous suggestions should still apply.
Also for the future if you could avoid rebasing and add commits instead -- it's easier to see what changed since last review, and we squash commits when merging anyway. Thanks!
There was a problem hiding this comment.
This shouldn't be necessary in any case because it is already done by building the numpy package. So having it in the run requirements below covers it. Hopefully we will add build requirements and then numpy could be moved there.
|
This will also need to be fixed for #2551 but maybe we can merge this first and make @ryanking13 deal with it. |
|
Thanks @benjeffery! |
Sure, I can deal with it. Thanks @hoodmane and @benjeffery! |
Add the msprime package, a wieldy used genetic simulation tool.