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
15 stars 31 forks source link

Better error when packing empty page. #133

Closed danielballan closed 4 years ago

danielballan commented 4 years ago

Currently, calling pack_event_page() with no arguments hits an unexpected code path and thus fails like

UnboundLocalError: local variable 'event' referenced before assignment

This PR causes it to raise with a more describing ValueError. It is not possible to pack an "empty" Event Page because an Event Page must have a time and a uid.

The situation in the same for Datum Page, which must have a Resource. That is also addressed here.

Tests included, should be good to merge.

tacaswell commented 4 years ago

Per discussion on slack, the problem here isn't that we would have empty uids and times, it is that without at least one Event we can not infer the descriptor, To quote @danielballan

In summary, an empty EventPage is OK, but an EventPage will a NULL descriptor field is not OK.

I have a very weak preference that the wording in the exceptions be changed to reflect this, but will not block merging on that.