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

Allow DocRouter subclass to define page or non-page. #131

Closed danielballan closed 4 years ago

danielballan commented 4 years ago

Currently, DocumenterRouter routes any Events it sees to the event_page method, packing them into EventPages along the way. This means that if someone subclasses DocumentRouter and does something useful the event_page method, it will apply to event as well, automatically.

However, the reverse does not work. If a subclass defines event but not event_page, any Events will be handled but EventPages will pass through quietly.

This PR allows a DocumentRouter subclass to define event or event_page and have the documents flow to the one that is defined, working in the same way that __add__ and __radd__ do in Python. As before, the subclass may define both if it wishes (potentially useful for performance optimizations) or neither.

This PR needs careful tests.

danielballan commented 4 years ago

I would like @tacaswell to be a required reviewer here because I don't think we have ever used NotImplemented in our projects, and I have never used it personally, so I want a second opinion on whether this is overly clever.

tacaswell commented 4 years ago

Would it be simpler to not define the dispatched-to methods and watch for AttributeError?

danielballan commented 4 years ago

I think NotImplemented is a language feature that probably should be used more than it is. An AttributeError can mean "Um, what? Do you have the wrong object?" NotImplemented means "Reasonable question! But you are barking up the wrong tree." It's more explicit. @tacaswell also pointed out (via DM) that the NotImplemented involves less stack thrashing and allows child classes to "turn off" a parent class' implementation.

danielballan commented 4 years ago

Added tests.