search

bluesky / event-model

data model for event-based data collection and analysis
https://blueskyproject.io/event-model
BSD 3-Clause "New" or "Revised" License
13 stars 30 forks source link

Add projections schema to start doc #179

Closed dylanmcreynolds closed 4 years ago

dylanmcreynolds commented 4 years ago

Description

This is a take on another PR related to "techniques", which turned into "projections". https://github.com/bluesky/event-model/pull/130. I considered trying to commit to that PR, but this seemed cleaner, even though 130 has a lot of critical discussion in it. This really isn't very different from that.

Motivation and Context

I won't got into too much detail here, but the basic thought was to address the need for mapping from a beamline's ontology to one ore more ontologies. I had in mind a case where a beamline is doing a scattering experiment and wants to provide the map to downstream users/application about how to map fields in NXsas fields. with that in mind, I had a few things in mind in the design:

I took no position on the names of Projections. For this all to work in, say, the NeXuS case, there will still need to be some sort of agreed format for specifying the fields to do this mapping. But I felt that that "unique string" was all we need for this (uniqueness being enforced by being property names in the Projection object.

EDIT: I also updated compose_run to allow for projections to be written to the root of a run_start doc.

How Has This Been Tested?

Added new test_projecitonpy including at least one failing case (which exposed a nice bug in my initial schema attempt).

dylanmcreynolds commented 4 years ago

@tacaswell and @danielballan, I debated adding to #130 vs. a whole new PR, especially since what we came up with was so similar. But if it was better in Tom's PR, then I suppose it's just one commit away.

jklynch commented 4 years ago

The test_em.py file is very long. I think projections tests should get their very own test file test_projections.py.

dylanmcreynolds commented 4 years ago

I changed compose_run to actually allow a projection to be written to the run_start document.

@jklynch While adding a test, moved stuff out of test_em.py and into new test_projections.py

dylanmcreynolds commented 4 years ago

I added calculation fields.

There is a corresponding PR for a projector in databroker that uses these projections: https://github.com/bluesky/databroker/pull/585

dylanmcreynolds commented 4 years ago

Andi points out a concern that the the start document is already getting big at some beamlines, and that adding this to it might get in the way of searching.

danielballan commented 4 years ago

@dylanmcreynolds Please add mention of this to the documentation, probably in this section, https://blueskyproject.io/event-model/data-model.html#run-start-document and note clearly that this is experimental.

Note: On today's pilot call, we agree to merge this (marked "experimental") by a week from today.

dylanmcreynolds commented 4 years ago

@dylanmcreynolds Please add mention of this to the documentation, probably in this section, https://blueskyproject.io/event-model/data-model.html#run-start-document and note clearly that this is experimental.

Note: On today's pilot call, we agree to merge this (marked "experimental") by a week from today.

Committed a brief note there.

danielballan commented 4 years ago

We agreed to merge this in order to lower the barrier for people to play with it, on the understanding that we will very likely make backward-incompatible changes as we gain more experience with it.

Just waiting flake8 fixes, then good to go! :rocket:

dylanmcreynolds commented 4 years ago

I bumped the version of jsonschema from 2 to 3 in the travis run that pegs jsonschema version and it runs. I don't know if this is appropriate or if it's better to remove that test run. @tacaswell , @danielballan ?

danielballan commented 4 years ago

[Already covered this on Slack but re-summarizing for the public record:]

The build that pins jsonschema==2 is meant to protect against the problem described in https://github.com/bluesky/event-model/issues/145, and it should stay until we need some feature in a newer version of jsonschema. I force-pushed to remove the commit that altered the CI configuration. It turns out that jsconschema==2 was finding a minor mistake in the schema that was for whatever reason ignored by jsonschema 3.x. Fixed in 2ca0aec.