Conversation
|
looks good at first glance. |
|
@mrosseel I added it in alphabetical order in the DSOs list. That feels most correct as everything else was in alphabetic order. Are you seeing something different? I do agree that we are getting a number of catalogs and should revisit the order. |
|
Fantastic @casazza ! Thank you again for another great catalog addition. Did you happen to have a bead on the usage/license for this? I didn't see this in the readme and I tried to check the source URL and it looks like the server is down. |
|
@brickbots This was free to use. No special notification or attribution required. I added it to the Catalog's doc. That should be more than sufficient. |
| return designation, is_official | ||
|
|
||
|
|
||
| def identify_catalog_type(name: str) -> Tuple[bool, str]: |
There was a problem hiding this comment.
there are already common facilities to find if something is in our database called ObjectFinder, so maybe you can use those for this use case? Or extend them / add a utility function if you have a unique use case
|
just have a look at that one remark I gave but for the rest it looks good, haven't tested it locally yet though. |
|
@mrosseel Sorry for the long delay. I have been getting the final push of the new version of TonightsSky through beta. I will get a chance to look at this later this week or weekend. |
|
@mrosseel identify_catalog_type serves a different purpose than ObjectFinder. ObjectFinder resolves whether an object already exists in the database by designation — it's a deduplication tool. identify_catalog_type classifies whether a name string is a PiFinder-recognized catalog prefix (NGC, IC, M, etc.) so it can be routed to aka_names for cross-matching versus common_names for the names table. These are two distinct operations. That said, if the project would prefer this classification list live in a shared location (e.g. a constant in catalog_import_utils.py) rather than repeated per loader, I'm happy to refactor it there — harris_loader.py has the same list and it could be consolidated. |
Lynga Open Cluster Catalog loader.
Unit tests needed change to allow for 1 object at a Dec or exactly 0.0