bioio-devs / bioio-bioformats

A BioIO reader plugin for reading bioformats formatted images.
GNU General Public License v3.0
0 stars 0 forks source link

pin numpy #14

Open BrianWhitneyAI opened 2 months ago

BrianWhitneyAI commented 2 months ago

Core Issue

The purpose of this PR is to investigate some improper metadata reading for the bioformats reader. As can be seen here . The channels for the Imaris image are being read as ['Red', 'Green', 'Blue'] rather than ['Channel:0:0', 'Channel:0:1','Channel:0:2']. The weird part Is that I cannot replicate this locally. I originally assumed that this was a dependency issue ( This is what led me to pin Numpy, see below). However with the exact dependencies and confirming that the download hash is the same for test files I still have not been able to replicate this locally.

Notes

1) numpy 2.0.0 moves from np.round to np.round. dask.array still uses np.round raising AttributeError: np.round_ was removed in the NumPy 2.0 release. Use np.round instead. I have pinned Numpy to look at the deeper issue. 2) This issue seems to have been seen before here #10 however the logs have expired so I cannot tell 100% that it is the same issue.

evamaxfield commented 2 months ago

I would argue that we should do something like:

import numpy as np

def np_round(*args, **kwargs) -> int,
    if np.__version__.split(".")[0] == "1":
        return np.round_(*args, **kwargs)

    return np.round(*args, **kwargs)

Because pinning numpy should be imo a last resort, that might break a lot of things.

BrianWhitneyAI commented 2 months ago

I would argue that we should do something like:

import numpy as np

def np_round(*args, **kwargs) -> int,
    if np.__version__.split(".")[0] == "1":
        return np.round_(*args, **kwargs)

    return np.round(*args, **kwargs)

Because pinning numpy should be imo a last resort, that might break a lot of things.

Still in draft! I agree this would be better. There's still some weird things going on here with tests, the pin is to see the lower error which I cannot see locally.

BrianWhitneyAI commented 2 months ago

@evamaxfield @toloudis I wanted to see if either of you had any insight into this. Tests fail only in GH Action and neither Sean or I can replicate.

toloudis commented 2 months ago

First guess: maybe check version differences between what bioformats is being installed on your local machine vs gh-actions?

Is it possible bioformats fixed something and the new behavior is actually more correct?

It doesn't seem like this could be numpy, as bioformats is a pure java implementation

BrianWhitneyAI commented 2 months ago

First guess: maybe check version differences between what bioformats is being installed on your local machine vs gh-actions?

Is it possible bioformats fixed something and the new behavior is actually more correct?

It doesn't seem like this could be numpy, as bioformats is a pure java implementation

It is not numpy. I just pinned numpy to be able to see the error again since I cannot reproduce locally. I have the same versions installed locally as is described in the action. maybe we have a cached version that has a hotfix somewhere that wasnt annotated with a versioning?