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 EventList.select_map_mask() method #2049

Merged
merged 5 commits into from Feb 28, 2019

Conversation

Projects
None yet
2 participants
@registerrier
Copy link
Contributor

registerrier commented Feb 26, 2019

This PR introduces a selection on EventListBase based on Map defined masks.

This will allow more detailed selections in particular regarding reflected region.

@registerrier registerrier requested a review from adonath Feb 26, 2019

@registerrier registerrier self-assigned this Feb 26, 2019

@registerrier registerrier added this to the 0.11 milestone Feb 26, 2019

@adonath
Copy link
Member

adonath left a comment

Thanks @registerrier! I've left a few in-line comments...


return MapCoord.create(coord, coordsys="CEL")

def select_from_map_mask(self, mask):

This comment has been minimized.

Copy link
@adonath

adonath Feb 27, 2019

Member

Maybe rename to select_map_mask, to be consistent with other select_... methods?

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 27, 2019

Author Contributor

Done

----------
mask : `~gammapy.maps.Map`
the mask to be used
Returns

This comment has been minimized.

Copy link
@adonath

adonath Feb 27, 2019

Member

There is an empty line missing...

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 27, 2019

Author Contributor

Done

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 27, 2019

Author Contributor

Done

----------
geom : `~gammapy.maps.MapGeom`
the geom used to define the MapCoord
Returns

This comment has been minimized.

Copy link
@adonath

adonath Feb 27, 2019

Member

Missing empty line...

except KeyError:
raise KeyError("Column not found in event list: {!r}".format(axis.name))

return MapCoord.create(coord, coordsys="CEL")

This comment has been minimized.

Copy link
@adonath

adonath Feb 27, 2019

Member

I'd prefer to just return a dict of {"skycoord": self.radec, "energy": energy, "...": ..., } to avoid the MapCoord import. The dict can be passed to .get_by_coord() as well.

registerrier added some commits Feb 27, 2019

@registerrier registerrier requested a review from adonath Feb 27, 2019

@adonath
Copy link
Member

adonath left a comment

Thanks @registerrier, I have no further comments.

@registerrier registerrier merged commit 1110efd into gammapy:master Feb 28, 2019

3 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: 4 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@adonath adonath changed the title Add an EventList selection based on Map Add EventList.select_map_mask() method Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.