admin/remove-bioformats-extra#329
Conversation
Codecov Report
@@ Coverage Diff @@
## main #329 +/- ##
==========================================
- Coverage 94.46% 94.30% -0.17%
==========================================
Files 40 40
Lines 3018 3018
==========================================
- Hits 2851 2846 -5
- Misses 167 172 +5
Continue to review full report at Codecov.
|
evamaxfield
left a comment
evamaxfield
left a comment
There was a problem hiding this comment.
Works for me. @toloudis we should make these changes to CZI + LIF as well unfortunately.
That would be fine. I still don't know what to do with the license/docs for aicspylibczi :) |
|
Going to wait for approval from one of @chris-allan, @joshmoore, @sbesson, @jrswedlow before merge. |
AetherUnbound
left a comment
AetherUnbound
left a comment
There was a problem hiding this comment.
Dang, this is unfortunate but it's solid that we're respecting the licenses appropriately.
| "lif": ["readlif>=0.6.4"], | ||
| "czi": ["aicspylibczi>=3.0.2"], | ||
| "bioformats": ["bioformats_jar", "wrapt>=1.12"], | ||
| # "bioformats": ["bioformats_jar"], # excluded for licensing reasons |
There was a problem hiding this comment.
I like the idea of leaving a note in here for future contributors (in addition to the other docs), but I'm not sure it needs to keep this form. Does this work?
| # "bioformats": ["bioformats_jar"], # excluded for licensing reasons | |
| # bioformats_jar ("bioformats") excluded for licensing reasons |
sbesson
left a comment
sbesson
left a comment
There was a problem hiding this comment.
Many thanks @tlambert03 for opening this PR on this complex question.
I had forgotten to mention it in the initial PR but the closest scenario of the Bio-Formats side is the GNU Octave package where the wrappers are released under a BSD 2-clause license but the usage requires the separate installation of bioformats_package.jar - see the corresponding installation instructions.
From my side, the set of changes here align well with the scenario above and provide a good compromise between maintaining Bio-Formats support in aicsimageio and reducing the ambiguity and the risks associated with the extra_requirements usage. Users can still choose to install an aicsimagio-compatible Bio-Formats bindings in their environment using an user-friendly pip based workflow so the overhead is fairly minimal compared to the previous solution. On the other side, the requirement for the explicit installation of a third-party package is clarified both at the code and documentation level so that downstream consumers are aware of the licensing implications.
lets additionally please wait for approval from @jrswedlow and @chris-allan |
Agree. Want to be thorough here. |
|
Thanks for all the hard work and care all. Changes look great to me. |
|
Thumbs up from my side. Thanks much to the whole team for the work and consideration. |
8 participants
Description
As per request from the OME team, this removes the
[bioformats]extra and instructs users to install it manually withpip install bioformats_jar. cc @chris-allan, @joshmoore, @sbesson, @jrswedlow ... let us know if this is what you had in mind.Pull request recommendations: