fmicompbio / ez_zarr

GIve easy, high-level access to ome-zarr fileset
https://fmicompbio.github.io/ez_zarr/
MIT License
13 stars 0 forks source link

add **kwargs for keyword argument to be passed to `plotting.plot_image` #6

Closed silvbarb closed 5 months ago

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (0fa2895) 100.00% compared to head (ea9d309) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 4 4 Lines 976 976 ========================================= Hits 976 976 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

silvbarb commented 6 months ago

Yes, I see your point, thanks for spotting this!

Even if in some cases we have avoided **kwargs, and explicitly exposed the arguments that can be internally passed (e.g. pyramid_level_coord added to plot_well, to be passed to get_image_ROI and get_label_ROI), this option makes the argument list of the outer function very long, and potentially more confusing to the user.

Which approach would you suggest? How about we filter the allowed **kwargs in the outer function? Something like:

def outer_function(arg1, arg2, **kwargs):
    allowed_kwargs = {"allowed_arg1", "allowed_arg2"}
    filtered_kwargs = {k: v for k, v in kwargs.items() if k in allowed_kwargs}

    inner_function(arg1, arg2, **filtered_kwargs) 

in this way we can throw out im and keep call_show, but in the future we can add other arguments to the allowed ones if needed.

csoneson commented 6 months ago

Yes, I think that would work - I'm not sure if it's easier to specify the allowed ones or the forbidden ones (the latter would be the ones that are generated inside the outer function, and one could perhaps imagine initializing a list in the beginning of the outer function and adding to it whenever defining an argument). I think it would be helpful to also alert the user of which of their provided arguments were in fact not passed on (and why).

mbstadler commented 5 months ago

Thanks to both for the useful discussion. As decided together, we'll for the moment expose call_show from plot_image in plot_well and close this PR