-
Notifications
You must be signed in to change notification settings - Fork 51
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
adding h5py recipe #685
adding h5py recipe #685
Conversation
Thanks for the recipe! |
Many thanks @DerThorsten for the commits looks like I am getting a error on the build the recipe on this pr looks similar to the conda-forge recipe so I'm guess that mamba can't find the same range of packages in in emscripten-forge.
|
Jeah just try without any pinning in the beginning! |
interesting, from the CI error it looks like I'm missing channels for the install
|
I updated the pinning of hdf5 in the global build config, this should fix that issue |
@shimwell we probably need some patches. Probably these here https://github.com/pyodide/pyodide/tree/main/packages/h5py/patches |
Thanks for working on this @DerThorsten I have had a go at adding the patch. I've never actually made use of a git patch before so hope I've done it correctly. Thanks for teaching me |
Would it be possible for someone with permissions to approve the ci tests |
Looks like the cython failed in the CI Error compiling Cython file: |
The Cythonizing errors look a bit beyond me, is this the sort of thing you were thinking about @aragilar and @tacaswell when you predicted changes in the code base / fixes upstream on this issue
|
fyi for complicated debugging, it is best to install the building machinery locally. |
You may need to pin cython to |
Adding the Current errors are now ... shared:INFO: (Emscripten: Running sanity checks)
/home/runner/micromamba/envs/ci/conda-bld/h5py-0_1700504180033/work/h5py/defs.c:10425:85: warning: incompatible pointer types passing '__pyx_t_5numpy_uint32_t *' (aka 'unsigned long *') to parameter of type 'uint32_t *' (aka 'unsigned int *') [-Wincompatible-pointer-types]
10425 | __pyx_v_r = H5Dread_chunk(__pyx_v_dset_id, __pyx_v_dxpl_id, __pyx_v_offset, __pyx_v_filters, __pyx_v_buf);
| ^~~~~~~~~~~~~~~
/home/runner/micromamba/envs/ci/conda-bld/h5py-0_1700504180033/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/include/H5Dpublic.h:1011:92: note: passing argument to parameter 'filters' here
1011 | H5_DLL herr_t H5Dread_chunk(hid_t dset_id, hid_t dxpl_id, const hsize_t *offset, uint32_t *filters,
| ^
/home/runner/micromamba/envs/ci/conda-bld/h5py-0_1700504180033/work/h5py/defs.c:26245:15: error: call to undeclared function 'H5Pget_fapl_direct'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
26245 | __pyx_v_r = H5Pget_fapl_direct(__pyx_v_fapl_id, __pyx_v_boundary, __pyx_v_block_size, __pyx_v_cbuf_size);
| ^
/home/runner/micromamba/envs/ci/conda-bld/h5py-0_1700504180033/work/h5py/defs.c:26245:15: note: did you mean 'H5Pget_fapl_core'?
/home/runner/micromamba/envs/ci/conda-bld/h5py-0_1700504180033/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/include/H5FDcore.h:92:15: note: 'H5Pget_fapl_core' declared here
92 | H5_DLL herr_t H5Pget_fapl_core(hid_t fapl_id, size_t *increment /*out*/, hbool_t *backing_store /*out*/);
| ^
/home/runner/micromamba/envs/ci/conda-bld/h5py-0_1700504180033/work/h5py/defs.c:26375:15: error: call to undeclared function 'H5Pset_fapl_direct'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
26375 | __pyx_v_r = H5Pset_fapl_direct(__pyx_v_fapl_id, __pyx_v_alignment, __pyx_v_block_size, __pyx_v_cbuf_size);
| ^
/home/runner/micromamba/envs/ci/conda-bld/h5py-0_1700504180033/work/h5py/defs.c:27675:13: warning: assigning to 'void *' from 'const void *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
27675 | __pyx_v_r = H5Pget_driver_info(__pyx_v_plist_id);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings and 2 errors generated. |
class HDF5LibWrapper: | ||
|
||
def __init__(self, libdirs): | ||
- self._load_hdf5_lib(libdirs) |
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 not chased through how, but I think that this patch is breaking all of h5py's auto-detection of what capabilities the wrapped libhdf5 has.
Co-authored-by: Thomas A Caswell <[email protected]>
Nice, that |
h5py is adding runtime_library_dirs to the build_ext command when os.name != 'nt', which not compatible with emscripten. I added another patch, and activated the patch in the recipe.yaml ... with the patch below, the h5py library builds, though there is an error during testing at the end due to deprecated usage of ruamel in boa (?) diff --git a/recipes/recipes_emscripten/h5py/patches/configure.patch b/recipes/recipes_emscripten/h5py/patches/configure.patch
index 29bcada..382f800 100644
--- a/recipes/recipes_emscripten/h5py/patches/configure.patch
+++ b/recipes/recipes_emscripten/h5py/patches/configure.patch
@@ -60,6 +60,27 @@ index c16ddeef..1180d3fa 100644
def has_direct_vfd_support(self):
return self.has_functions("H5Pget_fapl_direct", "H5Pset_fapl_direct")
+diff --git a/setup_build.py b/setup_build.py
+index 966fdc97..ecdb6738 100644
+--- a/setup_build.py
++++ b/setup_build.py
+@@ -12,6 +12,7 @@ except ImportError:
+ from distutils.command.build_ext import build_ext
+ import sys
+ import os
++import platform
+ import os.path as op
+ from pathlib import Path
+
+@@ -112,7 +113,7 @@ class h5py_build_ext(build_ext):
+ settings['include_dirs'] += [mpi4py.get_include()]
+
+ # TODO: should this only be done on UNIX?
+- if os.name != 'nt':
++ if platform.system() == 'Linux':
+ settings['runtime_library_dirs'] = settings['library_dirs']
+
+ def make_extension(module):
--
2.35.1
diff --git a/recipes/recipes_emscripten/h5py/recipe.yaml b/recipes/recipes_emscripten/h5py/recipe.yaml
index c31af93..cda0586 100644
--- a/recipes/recipes_emscripten/h5py/recipe.yaml
+++ b/recipes/recipes_emscripten/h5py/recipe.yaml
@@ -10,6 +10,8 @@ source:
- url: https://github.com/h5py/h5py/archive/{{ version }}.tar.gz
sha256: 9cac7838ab139ec71adf721a1305b954e18efa4f80c82fcfe34e1eb9639ded29
- path: setup.py
+ - patches:
+ - patches/configure.patch
build:
number: 0
@@ -38,7 +40,7 @@ about:
home: https://www.h5py.org/
license: BSD 3-Clause
license_family: BSD 3-Clause
- license_file: LICENSE.txt
+ license_file: LICENSE
summary: Read and write HDF5 files from Python
description: |
The h5py package is a Pythonic interface to the HDF5 binary data format |
The error message during testing: AttributeError: yaml = YAML(typ='safe', pure=True) instead of file "/home/bbm/miniforge3/envs/emscripten-forge/lib/python3.10/site-packages/boa/core/test.py", line 67
|
I see the CI is making use of a boa fork which has the line you mentioned The boa repo has a different section that uses the YAML code you mentioned |
I can confirm that using a stock version of boa (installed v0.17 from mamba) that the build completes successfully, but no tests are run (I'm guessing that the patched version of boa does this correctly?) Here is the h5py library created: |
jeah we use this custom version of boa only for the purpose of being able to run the tests (since they run in a headless browser, its all a bit complicated) |
I just updated this branch and the CI failed with what I think is a new error. I can't see "safe_load" in the CI failing logs. I think this is the error now but I'm not totally sure.
|
I think that it is now failing on a link step:
|
Closing in favour of #1354 |
Attempting to add the h5py package to emscripten-forge.
I notice hdf5 is already on emscripten-forge perhaps it is therefore also good to add h5py.
I've not successful made a package for emscripten-forge before so I looked at a few others on the recipes folder and this is my first attempt.
Linking to the request as request on the h5py repo
h5py/h5py#2338