AllenCellModeling / aicsimageio

Image Reading, Metadata Conversion, and Image Writing for Microscopy Images in Python
https://allencellmodeling.github.io/aicsimageio
Other
207 stars 51 forks source link

anaconda distro #312

Closed toloudis closed 1 year ago

toloudis commented 3 years ago

Use Case

For some users it might be easier to conda install aicsimageio

Solution

support both pip and conda

tlambert03 commented 3 years ago

I can add this if someone isn't already working on it. (Note, we'll need to add each plugin as well... and since there are no "extras" in conda, I propose that the aicsimageio package installs all the formats currently available on conda)

tlambert03 commented 3 years ago

working on it here: https://github.com/conda-forge/staged-recipes/pull/16232

evamaxfield commented 3 years ago

Thanks for jumping on this one @tlambert03

toloudis commented 3 years ago

Yes, thanks! My question is how does it get automated for aicsimageio releases? Does something happen from the aicsimageio github actions?

tlambert03 commented 3 years ago

answered here: https://github.com/conda-forge/staged-recipes/pull/16232#issuecomment-925151937

it will all make sense :) once you see a release cycle or two. You'll be added as a member to a new repository and get pinged whenever the bots detect a new version available on PyPI. so you just continue releasing here as usual

tlambert03 commented 3 years ago

PS, leave a note over there saying "I'm happy to be listed as a maintainer"

evamaxfield commented 3 years ago

Going to close this because its all setup for our 4.1 release correct?

tlambert03 commented 3 years ago

Indeed! https://anaconda.org/conda-forge/aicsimageio

psobolewskiPhD commented 3 years ago

This is just the base version right? so for [lif] or [czi] one would still use pip?

tlambert03 commented 3 years ago

Yes, those packages all need to be put on conda forge separately. And once they're there, because conda doesn't have extras, I'd propose that they be included as dependencies of aicsimageio by default. This is similar to what other packages do (for instance conda install dask brings down everything needed for dask[array] etc)

evamaxfield commented 3 years ago

Oh in that case going to leave this open until that is done too.

tlambert03 commented 3 years ago

probably going to need a little help on the czi build. took a look at it today and had some questions. who knows the compile chain there best... is it @toloudis? Is the ultimate goal to merge aicspylibczi back with the parent repo? should we be creating an anaconda package called aicspylibczi or pylibczi? Also, it would be helpful (if not strictly necessary) if the sdist was included on pypi.

evamaxfield commented 3 years ago

I just opened this: https://github.com/AllenCellModeling/aicspylibczi/pull/89

Hopefully we can merge that PR in next week to simplify the whole build process A LOT. The person that knows it the best would be Jamie but he no longer works at AICS / works on the project. I know very little about building of C++ libs / pybind11 so I defer to @toloudis on that.

Feel free to comment on that PR of any changes you think we should make to the build process.

tlambert03 commented 3 years ago

great thanks!

toloudis commented 3 years ago

After AllenCellModeling/aicspylibczi#89, do we need to do anything special for conda support? Is it just a matter of the same kind of thing as the bioformats and readlif ones? Let us know what questions remain.

tlambert03 commented 3 years ago

this one is a lot harder... because you can't just copy over the prebuilt wheels to conda... you need to actually build it with conda-build. every time i do a compiled project on conda, I need to read up on it a bit. so will have to get back to you

tlambert03 commented 3 years ago

worked on this a bit more (PR to staged-recipes here).

The current issues surround getting the pybind11 and libczi dependencies in for the build process. the repo is setup for a git clone --recurse-submodules style build ... and the sdists on pypi and the release tarballs aren't "build-ready" (understandably). conda forge discourages building from repos though, and generally requires building from versioned tarballs (also understandably). While there is a pybind11-abi dependency that I think will work, we probably need to make a more complicated build.sh to prep this package for building.

tlambert03 commented 3 years ago

@toloudis ... i have czilib building for mac and linux, but hitting this error on windows

  -- Building for: NMake Makefiles
  CMake Error at CMakeLists.txt:9 (project):
    Generator

      NMake Makefiles

    does not support platform specification, but platform

      x64

    was specified.

  CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
  CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage

it seems perhaps related to this line in setup.py. I might need to add some additional dependency to the build environment? or perhaps I need to set some env var in the bld.bat script here.

toloudis commented 3 years ago

Not that this is necessarily your fix, but I think the git clones in the bld.bat are not dialing in the right version of pybind11 - I think we had it set to a particular release commit.

toloudis commented 3 years ago

I suspect that cmake is looking for visual studio generator, not finding it (for some unknown reason), and using nmake makefiles as the default generator on that windows system.

Slightly hesitant to suggest this but a possible fix for "NMake Makefiles does not support platform specification, but platform x64 was specified." is really just removing that CMake generator platform line in setup.py. This depends on the selection of x64 might happen automatically if the rest of the environment is set up for it -- so it may break the non-nmake case.

Normally when I build a project with CMake I use the Visual Studio generator and not NMake makefiles. This is one of those "try it and maybe it will work" things. (is it easy to test? Can we put it on a branch and have the conda forge setup pull that branch?)

tlambert03 commented 3 years ago

is it easy to test? Can we put it on a branch and have the conda forge setup pull that branch?

sure, but this can all be tested locally too: https://conda-forge.org/docs/maintainer/adding_pkgs.html#staging-test-locally

evamaxfield commented 3 years ago

I believe we now need to update the license on the conda distro to GPL. / We should decide which packages are included in the distro.

tlambert03 commented 3 years ago

sorry, before seeing this I just removed readlif and bioformats_jar from the 4.3 conda distro... so someone would need to do conda install aicsimageio readlif bioformats_jar now... but I like the idea of just relicensing that distro

evamaxfield commented 3 years ago

Yep. Similar to napari-aicsimageio, it would be great to have CI build two of them. One for GPL and one for BSD

tlambert03 commented 3 years ago

I think for that we'd need to submit a new PR to conda forge staged recipes, with a new name (aicsimageio-gpl or something), with a new recipe. I don't think you can have a single feedstock create two packages with different recipes/licenses

toloudis commented 3 years ago

is it easy to test? Can we put it on a branch and have the conda forge setup pull that branch?

sure, but this can all be tested locally too: https://conda-forge.org/docs/maintainer/adding_pkgs.html#staging-test-locally

I just tried this: cloned your fork (the aicspylibczi branch) on an ubuntu machine, used CONFIG=win64 as specified in the link above, and it seems to have built without error!

SeanLeRoy commented 1 year ago

@tlambert03 @evamaxfield Does this seem like it has been completed based on @toloudis's test and this issue status?

evamaxfield commented 1 year ago

Honestly, I have no idea if this can be closed.

For some reason I thought we were still having problems with the conda release of aicsimageio (disregarding the desire to have aicsimageio-gpl and aicsimageio-bsd releases)

I feel like it was something like czi reading failed with the conda installation of aicsimageio...

It might be good to just locally make a new conda environment, install aicsimageio via the conda, and then run the test suite. see what breaks. If nothing breaks, feel free to close this.

SeanLeRoy commented 1 year ago

With aicsimageiomoving into "maintenance" mode where only easy to implement bugfixes deemed critical (or community contributed) work being done in aicsimageio, this seems like it should be closed for aicsimageio and opened as an issue in bioio. I'll close this issue and create the aforementioned mirror issue in bioio linking the two, but if this seems important to keep open feel free to re-open of course.

Edit: See issue in bioio