Another structure to consider before we settle on one

[danielballan]

A thought I had on the way home that I just want to scribble down for discussion tomorrow:

The availability of event_model.DocumentRouter, which landed after we started work on suitcase-csv, invites another promising possibility for how to structure suitcases. We could move the business logic into a DocumentRouter subclass, trading the wall of if name == ... blocks for corresponding methods on DocumentRouter. We would still include a top-level function that consumes a generator (export / whatever we name it). Only the internal implementation would change: that function would become a thin wrapper around the callable class. Both would be public.

def export(gen, directory, **kwargs):
    serializer = Serializer(directory, **kwargs)
    for item in gen:
       serializer(*item)
    return serializer.artifacts

class Serializer:
    def __init__(self, directory, **kwargs):

        if isinstance(directory, (str, Path)):
            # Write to ordinary files by default.
            self._writer = suitcase.utils.MultiFileWriter(directory)
        else:
            self._writer = directory
        self.artifacts = self._writer.artifacts  # Expose as public.

    ...

# The two layers also make room for a compromise in the naming
# (high-level vs low-level) but that's not the main point.

Let's consider the what this would make easy and what it would make hard.

Pro:

• Avoid the boiler plate of the if statements, especially the layout munging here:

          elif (name == 'event' or name == 'bulk_event' or
                name == 'event_page'):
              if name == 'event':  # convert event to an event_pages list
                  event_pages = [event_model.pack_event_page(doc)]
              elif name == 'bulk_event':  # convert bulk_event to event_pages
                  event_pages = event_model.bulk_events_to_event_pages(doc)
              else:  # convert an event_page to an event_pages list.
                  event_pages = [doc]
              for event_page in event_pages:
                  event_model.verify_filled(event_page)
                  stream_name = stream_names[event_page['descriptor']]
                  ...

  ...which DocumentRouter does for free, munging any events and bulk_events internally and then routing through itsevent_page method.

   def event_page(self, doc):
      event_model.verify_filled(doc)
      stream_name = self.stream_names[doc['descriptor']]
      ...

  I expect this would appear much more sensible to prospective suitcase authors unfamiliar with the arcana of the event model's history.

• The DocumentRouter (a.k.a CallbackBase) pattern is one we already use extensively in bluesky and try to teach intermediate/advanced users who want to write custom live data processing or visualization code. It would be nice to have a consistent pattern for export --- fewer conventions to learn.
• With a class, we have the option to push documents in, go do something else, and then push more documents in. With a function, the entire generator is consumed all at once. (You may think of this as coroutine-like behavior, but with more flexibility and implemented with an arguably more widely-known language feature.) In this way the class is more flexible and a more direct fit for the use case of live export.

Con:

• The use of context managers is harder because an open context cannot bridge methods, and this means safe cleanup of the resources (closing files) has to be managed correctly manually. But in practice so far we haven't been able to use the context managers anyway.

Is this worth trying to see how it compares?

[awalter-bnl]

I like this approach mainly as it reduces the amount of 'duplicate' code we are seeing at the moment in the suitcase.* files. As you say it also gives both naming conventions, the higher level one and the lower level one which I also find appealing.

[awalter-bnl]

This has been resolved with PR#1