Initial feedback

[joshmoore]

Hi @tlambert03.

I gave napari-omero a try yesterday. Very nice! I'm bundling together a couple of issues I ran into for general discussion, but I'm happy to break them out into separate issues moving forward.

• I ran into an exception pointing to SessionsStore.ctx if I didn't pre-login from the CLI. This looks to be an assumption somewhere deeper in the omero-py code, but trying to mix pdb and Qt was less than successful. Do you always have an active login when running? This may be related to the security settings of IDR, but I don't have a clear smoking gun yet.

• I also note that a login appears to attempted before the user hits connect. I assume this is an attempt to re-use the previous session from session store? As a heads up, the CLI does this (and you've clearly drilled down pretty far into that code) but no other client does. It'd be interesting to hear your feedback.

• When running the project/dataset browser against the IDR, my instance seemed to go into an infinite loop. I haven't started to track this one down yet.

• Large images explode. I assume this would require loading tile by tile. I don't know if you have any plans to support that at the moment.

cc: @will-moore

[tlambert03]

Thanks a lot for the feedback!

exception pointing to SessionsStore.ctx ... Do you always have an active login when running?

handling of the client/gateway is definitely the thing I could use the most help with. This one likely calls for a separate issue, so I'll create one and give a partial answer there.

I also note that a login appears to attempted before the user hits connect. I assume this is an attempt to re-use the previous session from session store? As a heads up, the CLI does this but no other client does.

Yeah, I was mostly just looking for a way not to have to enter my password every time! 😂. I kinda like the behavior, but I'm definitely open to feedback on that. If you think it's security issue, we could make it opt-in somehow. At the very least, we need a "logout" button... though that is made slightly more complicated by the fact that the lazy dask arrays require the connection, so we'd basically need to either delete all currently-loaded layers with dask data (or convert them to local arrays) in order to avoid crashes after logging out.

When running the project/dataset browser against the IDR

I've never really used it... is that something I can try without a login? or can I create a login?

Large images explode. I assume this would require loading tile by tile.

def... it's a disaster at the moment. Yeah that would require tiling, so A) the dask chunk size here would need to be something other than the full XY plane and B) napari would need better 2D tiling support. Can the omero pixels API get a chunk/tile less than the full plane? or does that require something like zarr on the backend? as for tiling/chunking support in napari @pwinston is working hard on that sort of thing at the moment. So it's definitely a future goal.

[will-moore]

I've looked into supporting 'Large images' at https://github.com/tlambert03/napari-omero/pull/1 I create a dask delayed loader for each tile and concatenate them into each plane. But OMERO rawPixelsStore doesn't support pyramids, so I've had to use the rendering engine. So you get a 'rgb' image with the current OMERO rendering settings. Work in progress...

[tlambert03]

oh i see. very cool.

[joshmoore]

But OMERO rawPixelsStore doesn't support pyramids,

See https://github.com/tlambert03/napari-omero/pull/1#issuecomment-649410069 ?

[joshmoore]

Yeah, I was mostly just looking for a way not to have to enter my password every time! 😂. I kinda like the behavior, but I'm definitely open to feedback on that. If you think it's security issue, we could make it opt-in somehow.

I don't think it's a particular security issue. You've followed the CLI model (which I support: I'm to blame if anything is amiss there) but it's different from other GUIs, so just raising the flag.

[tlambert03]

Ok good to know. It’s an easy pattern to change in the future, so I’ll leave it for the time being

[tlambert03]

I ran into an exception pointing to SessionsStore.ctx if I didn't pre-login from the CLI. This looks to be an assumption somewhere deeper in the omero-py code

@joshmoore, looks like this is because I create a SessionStore here but do not provide it with a ctx attribute. however, in the SessionsStore.create method in omero-py, it assumes that a ctx has been injected into the class somewhere else in the code. Seems like it could be fixed on both ends... should I submit that to omero-py?

[joshmoore]

Those self.ctx. items look very much like a borked refactoring. If you have a fix, a PR against omero-py would be great.