ENH: changing the test utilities to pytest.fixtures

[awalter-bnl]

This PR creates the 'wrapper' functions to be used in the suitcase.*.export functions as well as adding some common pytest.fixtures for use in the tests for suitcase.*.export functions

[danielballan]

Looks close. I have many comments but no major ones.

[danielballan]

Why not simply collect all the documents? That seems like it would be a lot simpler. Then, in create_expected, you can loop over the list and select only the event variants.

[awalter-bnl]

event_list is also used inside this function when it is passed to event_model.pack_event_page(*event_list) which requires only a list of event documents not a list of all documents, it is not just passed externally to create_expected. As I have the list from that call I don't think it makes sense to create a new list only to extract this info out again inside create_expected.

[tacaswell]

I agree with Dan, having something that returns all of the documents and then picking out what you want in the tests seems much simpler. I would have expected this to look something like https://github.com/NSLS-II/bluesky/blob/7eea5f30d5e3a4c386585e23d17048b16cdac776/bluesky/tests/utils.py#L30-L46 but handling the extra documents.

[awalter-bnl]

I am going to push back again,

create_expected can not take in collector without adding lot of complexity to first search through and extract out event, bulk_events or event_page documents. It would then need to convert these to a list of event documents (or one of the other types) before starting to collate the information into the expected dictionary.

It makes no sense to me to do this when I have already created the list of events that I need in events_data. I see it as adding extra unnecessary code.

[tacaswell]

We should not return the same data twice from the fixture, that will lead to more confusion long term.

The data should be returned in a full-fidelity form that works across the full range of possible data (multiple streams, multiple keys per stream, multiple descriptors per stream) in a way that we can test any suitcase against.

We probably want to parameterize that object to have one of {event, event_page, bulk_event} and the same with {datum, bulk_datum, datum_page} (and parameterized by how big the pages are?).

Probably also makes sense to parameterize this by plan so as we add test cases they fan out nicely.

[danielballan]

Probably also makes sense to parameterize this by plan so as we add test cases they fan out nicely.

Good idea.

One way to illustrate the limits of this custom data structure is to use a plan that has two EventDescriptors for the 'primary' stream.

@bluesky.preprocessors.run_decorator({})
def plan():
    yield from trigger_and_read(det)
    yield from configure(det, {}, {})  # something like that, see docstring
    yield from trigger_and_read(det)

and of course one that has two streams:

@bluesky.preprocessors.baseline_decorator([det])
def plan():
    yield from count([det])

[awalter-bnl]

This is now a working version ready for review.

[danielballan]

Per conversation in group meeting, add close() on *Manager classes.

[awalter-bnl]

ready for review again

[danielballan]

My understanding of where we landed:

The current JSON layout cannot capture generic document streams with full fidelity. What full-fidelity alternatives exist?

1. The obvious "guaranteed to work" layout would be a list of the documents themselves, but it is agreed (by @awalter-bnl and @danielballan -- maybe by @tacaswell too?) that said structure is not easy to look at, either in its textual form or in some sort of browser once loaded into an application.
2. {'streams': {'stream_name': [event_page_for_desc1, event_page_for_desc2]}} where in the simple/common case there would be one event page in that list.
3. {'streams': {'stream_name': new_thing}} where new_thing de-normalizes the descriptor UIDs, providing a vector so that all the event pages for that stream can be concatenated while still retaining references to their individual descriptor UIDs

(3) has the virtue of being flatter; (2) has the virtues of normalization. One could conceivably even do both (suitcase-json-dog, suitcase-json-cat) but if we are using JSON as a common "sidecar" file for formats like TIFF we unfortunately need to settle on One True Way.

[danielballan]

I should add: @awalter-bnl is going to work on implementing (3), which is the shortest step from the current code to something that has full fidelity. Then he will beef up the tests as suggested by @tacaswell above. Then we will discuss if we are happy with (3) or need to keep discussing the final layout.

[awalter-bnl]

I have updated the test fixture to work with the new meta dictionary structure that supports multiple descriptors per stream, I am now looking at modifying the tests here and in suitcase-tiff PR #1 to test for such cases

[awalter-bnl]

closing because we can't agree on a path forward