BCDA-APS / gemviz

Data visualization for tiled
https://bcda-aps.github.io/gemviz/
Other
4 stars 0 forks source link

Refactor to one method? #123

Closed prjemian closed 1 year ago

prjemian commented 1 year ago

Since get_run_detectors, get_run_positioners and get_run_stream_names are basically all the same, shouldn't we have a single function to handle all the items that are lists of strings?

https://github.com/BCDA-APS/gemviz/blob/a85128e6a7e081b513b6c0dd028b7eb4b0a165ee/gemviz/resultwindow.py#L123-L126 https://github.com/BCDA-APS/gemviz/blob/a85128e6a7e081b513b6c0dd028b7eb4b0a165ee/gemviz/resultwindow.py#L128-L131 https://github.com/BCDA-APS/gemviz/blob/a85128e6a7e081b513b6c0dd028b7eb4b0a165ee/gemviz/resultwindow.py#L139-L142

_Originally posted by @rodolakis in https://github.com/BCDA-APS/gemviz/pull/72#discussion_r1294017406_

prjemian commented 1 year ago

Good idea. These are called in the else: clause here: https://github.com/BCDA-APS/gemviz/blob/a85128e6a7e081b513b6c0dd028b7eb4b0a165ee/gemviz/resultwindow.py#L63-L66

It might be a good application of functools.partial to add the additional parameters when defining the columns

            "Positioners": partial(self.get_str_list, run, "start", "positioners"),
            "Detectors": partial(self.get_str_list, run, "start", "detectors"),
            "Streams": partial(self.get_str_list, run, "summary", "stream_names"),

and

    def get_str_list(self, run, doc, key):
        """Return the document's key values as a list."""
        items = utils.get_md(run, doc, key, [])
        return ", ".join(items)
prjemian commented 1 year ago

Note that functools.partial inserts the additional arguments at the start of the argument list. When action(run) is called, the doc & key arguments will precede the run. Change the call signature to:

    def get_str_list(self, doc, key, run):
    ...