Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project Event Model Configurations #615

Merged

Conversation

dylanmcreynolds
Copy link
Contributor

This PR primarily enhances projections to add a projection type for capturing Event Descriptor Configuration field. It is tied to a PR in event model: bluesky/event-model#192

In addition to adding this field type, I refactored the project_xarray function, creating a Projector class that handles the mechanics of parsing a projection. It uses callbacks to communicate to a caller. The project_xarray function now uses the Projector class, and is now handling only the construction of a return xarray.Dataset.

@dylanmcreynolds
Copy link
Contributor Author

This PR is very dependent on event model work. I have a little more work to do here to update changes that were recently merged into event model, so let's not merge this until I've completed that.

Copy link
Member

@danielballan danielballan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See some notes in line. Should we mark these as experimental in the docstrings?

"""Produces an xarray Dataset by projecting the provided run. Selects projection based on
logic of get_run_projection().
class Projector():
"""Handles much of the inner workings of projecting a BlueskyRun by scanning the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sphinx will format this more nicely if you do

Single-line summary
<blank line>
Optional paragraph with more detail

All projections with "location"="configuration" will look in the start document
for metadata. Each field will be added to the return Dataset's attrs dictionary keyed
on projection key.
def project_xarray(run: BlueskyRun, *args, projection=None, projection_name=None, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swallowed? If so remove because this makes it easy for typos to pass unnoticed.

@dylanmcreynolds
Copy link
Contributor Author

Do you have a formal format for marking "experimental" or is it as simple as saying it in the docstring?

@danielballan
Copy link
Member

We just put it in the docstring. There has been a move toward more formality around this in some projects, but it's not yet widespread. Could consider it in the future.

@danielballan danielballan merged commit cfd7360 into bluesky:master Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants