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

Add Observation base class #2638

Closed
cdeil opened this issue Nov 29, 2019 · 1 comment
Closed

Add Observation base class #2638

cdeil opened this issue Nov 29, 2019 · 1 comment
Assignees
Milestone

Comments

@cdeil
Copy link
Contributor

cdeil commented Nov 29, 2019

I noticed in https://docs.gammapy.org/dev/api/gammapy.cube.MapDatasetMaker.html that all the docstrings say that they support DataStoreObservation.

Really now we have two classes that are (or should be) supported here and in other places where an observation is used:

I suggest to resolve this issue by introducing a base class, like this:

class Observation:
    """Observation base class."""
    # empty, nothing for now
    # will grow methods, might become an abc.ABC

class MemoryObservation(Observation):
    # exactly what we have now

class DataStoreObservation(Observation):
    # exactly what we have now

and to adapt docstrings like in MapDatasetMaker (and isinstance checks if we have any).

That's something that can be done in ~ 1 hour, and IMO is the right, achievable solution for v1.0.
The Observation and Observations parallels the design of Dataset and Datasets, where Dataset is a small base class: https://docs.gammapy.org/dev/api/gammapy.modeling.Dataset.html

@AtreyeeS @adonath @registerrier - Thoughts?

Anyone willing to send a PR? (otherwise I'd be happy to do it, if it's agreed that it's an improvement)

@cdeil cdeil added the feature label Nov 29, 2019
@cdeil cdeil added this to the 0.15 milestone Nov 29, 2019
@cdeil cdeil added this to To do in gammapy.data via automation Nov 29, 2019
@cdeil cdeil modified the milestones: 0.15, 1.0 Dec 3, 2019
@QRemy QRemy self-assigned this Jan 24, 2020
QRemy added a commit to QRemy/gammapy that referenced this issue Jan 28, 2020
- add Observation base class for DataStoreObservation and MemoryObservation as suggested in gammapy#2638
- change Observations inheritance to MutableSequence to gain .append(), .extend(), .pop(), .remove() methods as discussed in gammapy#2678
QRemy added a commit to QRemy/gammapy that referenced this issue Jan 28, 2020
- add Observation base class for DataStoreObservation and MemoryObservation as suggested in gammapy#2638
- change Observations inheritance to MutableSequence to gain .append(), .extend(), .pop(), .remove() methods as discussed in gammapy#2678
QRemy added a commit to QRemy/gammapy that referenced this issue Jan 30, 2020
- add Observation base class for DataStoreObservation and MemoryObservation as suggested in gammapy#2638
- change Observations inheritance to MutableSequence to gain .append(), .extend(), .pop(), .remove() methods as discussed in gammapy#2678
@adonath adonath self-assigned this Jan 30, 2020
@adonath adonath modified the milestones: 1.0, 0.16 Jan 30, 2020
@adonath
Copy link
Member

adonath commented Jan 30, 2020

Solved in #2753.

@adonath adonath closed this as completed Jan 30, 2020
gammapy.data automation moved this from To do to Done Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
gammapy.data
  
Done
Development

No branches or pull requests

3 participants