search

ska-sa / katpoint

Coordinate library for the MeerKAT project
BSD 3-Clause "New" or "Revised" License
1 stars 3 forks source link

Modernise Catalogue (Part One) #44

Closed ludwigschwardt closed 5 years ago

ludwigschwardt commented 5 years ago

Relax the constraints on a Catalogue so that it may contain different targets with the same name (or alias). Each target is still required to be unique (i.e. they should all have different description strings). This is mainly done to support katdal when opening observations with targets that intentionally share the same name (e.g. when imaging a calibrator, targets may differ only in tags).

The lookup dict changes from a mapping of string to Target to a mapping of string to a list of Targets (i.e. all targets with the given name).

When looking up a target by name, return the most recently added target with that name. The same goes for removing a target by name.

This implements JIRA ticket SR-1464 and closes #13.

The main work is done in the first commit, while the rest embellish a bit around the main theme.

ludwigschwardt commented 5 years ago

@bmerry, I just realised that I merged this PR without implementing the discussed functionality of raising an AmbiguousLookupError or similar when try to look up a target by name and finding two targets with that name.

The current behaviour is well documented, but is it worth implementing the exception approach now to avoid API flip-flop?

bmerry commented 5 years ago

I'll leave that up to you. I've got too much else on my plate right now to have strong feelings about it.