Allow a "no-image" mode for mosviz #502
Conversation
|
Since this was something I've requested, I've also tested this. It minimally works. I think the |
There was a problem hiding this comment.
Works for me, but I think I agree with @havok2063 that if the images are optional, the argument in load_data should be images=None so that it defaults to None instead of needing explicit input of None.
And as @javerbukh said, potentially removing the image viewer in this case is on our radar but was decided to be out of scope for this particular PR.
|
I agree that the images argument should be optional and default to None, but only if that is the most frequent way users are going to use MosViz. From my previous interaction with this app, I thought the intended default way is to be used with images (as when reading from a mos table), and in that case the no-images mode would be optional. Maybe we need PO advice here? |
|
I'm personally in the opinion that the software should try its best given what the user gives it. In that case, I'd be in opinion of not requiring |
|
I modified |
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "mosviz.load_data(spectra_1d, spectra_2d, images=images)\n", |
There was a problem hiding this comment.
Just a note that you can still use the old syntax here, you don't necessarily have to name the images argument. Maybe we actually want to keep this line as-is (I say, having already approved the PR)?
There was a problem hiding this comment.
On the other hand, writing it explicitly ensures the user gets the point that it's an optional argument.
The Mosviz example notebook exercises this mode.