AllenCellModeling / napari-aicsimageio

Multiple file format reading directly into napari using pure Python
GNU General Public License v3.0
33 stars 10 forks source link

admin/update-min-aicsimageio-ver-and-move-to-gpl3 #36

Closed evamaxfield closed 2 years ago

evamaxfield commented 3 years ago

Pull request recommendations:

Resolves #35

Sets the new license to GPL because we are upgrading to aicsimageio v4.4.0 and installing all optional reader deps. Additionally fixes and updates some general README info.

Tagging @psobolewskiPhD @tlambert03. Any thoughts?

Thanks for contributing!

evamaxfield commented 3 years ago

Ignoring the failing builds. Will rerun when 4.4.0 is actually pushed to pypi

codecov-commenter commented 3 years ago

Codecov Report

Merging #36 (6f04345) into main (7605974) will decrease coverage by 0.10%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
- Coverage   83.15%   83.05%   -0.11%     
==========================================
  Files           7        5       -2     
  Lines         184      177       -7     
==========================================
- Hits          153      147       -6     
+ Misses         31       30       -1     
Impacted Files Coverage Δ
napari_aicsimageio/tests/__init__.py
napari_aicsimageio/__init__.py

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7605974...6f04345. Read the comment docs.

psobolewskiPhD commented 3 years ago

First, I'm incredibly flattered you listed me as a contributor in AICS 4.3 when I made a just a tiny tweak to a thing that in the same release became not a thing. 🤣

I can't say I'm fully able to wrap my head around the GPL vs. BSD etc, issues. That said, I do think that making all the readers available via the napari GUI would be a huge boon to users, including people I work with (I do understand bio-formats still requires a Java install, which can be a sticking point, but that's sort of a fallback anyways.) So I guess I come back to @tlambert03 comment about making this plugin a dependency for napari at some point. If GPL license—which I understand sort of propagates?—would make that impossible, then long-term this could be a negative. But if that's a long way off...

tlambert03 commented 3 years ago

yeah, I'm fine with this. If it later becomes a sticking point, we can create a BSD version at that time

evamaxfield commented 3 years ago

Holding for a week. Allen Legal looking into the ramifications of converting to a GPL license. (I.e. "how easy is it to move back to BSD after switching to GPL)

psobolewskiPhD commented 2 years ago

OK, so this is sort of insane, but it works... What if, the error from napari-aicsimageio when LIF or CZI is tried, but the extras are not installed (in other words, the default install, without the GPL parts) told the use to open the console and do: %pip install readlif or %pip install aicspylibczi (this is from: https://github.com/ipython/ipython/pull/11524) It works! You don't even need to relaunch napari! You can see console output and the scene selector of a LIF.

image

Edit: here's pip show from the terminal, showing that it installed in the right env.

─ ~/dev ······················································· ✔  3m 44s ─╮
╰─ pip show readlif                                               (napari-CL) ─╯
Name: readlif
Version: 0.6.5
Summary: Fast Leica LIF file reader written in python
Home-page: https://github.com/nimne/readlif
Author: Nick Negretti
Author-email: nick.negretti@gmail.com
License: GPLv3
Location: /Users/piotrsobolewski/Dev/miniforge3/envs/napari-CL/lib/python3.9/site-packages
Requires: Pillow
Required-by: 
evamaxfield commented 2 years ago

OK, so this is sort of insane, but it works... What if, the error from napari-aicsimageio when LIF or CZI is tried, but the extras are not installed (in other words, the default install, without the GPL parts) told the use to open the console and do: %pip install readlif or %pip install aicspylibczi (this is from: ipython/ipython#11524) It works! You don't even need to relaunch napari! You can see console output and the scene selector of a LIF. image

Edit: here's pip show from the terminal, showing that it installed in the right env.

─ ~/dev ······················································· ✔  3m 44s ─╮
╰─ pip show readlif                                               (napari-CL) ─╯
Name: readlif
Version: 0.6.5
Summary: Fast Leica LIF file reader written in python
Home-page: https://github.com/nimne/readlif
Author: Nick Negretti
Author-email: nick.negretti@gmail.com
License: GPLv3
Location: /Users/piotrsobolewski/Dev/miniforge3/envs/napari-CL/lib/python3.9/site-packages
Requires: Pillow
Required-by: 

Uhhhhhh can you make a PR for that?

evamaxfield commented 2 years ago

Errr wait, this PR is adding all the installs. No reason to add that once this is merged and deployed

evamaxfield commented 2 years ago

I have no idea why the tests are failing on Windows... but I am going to ship this and release so that it unblocks a lot of people.

Will make an issue for fixing windows tests.

evamaxfield commented 2 years ago

@psobolewskiPhD release is out v0.5.0.

If you have a chance, can you make a fresh env and let me know if everything installs and runs correctly.

I am technically out of town this week so will be sporadic in response but hopefully everything works for you!

psobolewskiPhD commented 2 years ago

Everything is golden! Works perfectly, both readlif and aicspylibczi.