fix incorrect asteroid-comet classification bug#422
Conversation
Added a line to disallow comet names that have a number in the second position in order to avoid having parse_comet() incorrectly interpret input like "2015XN77" as a well-formed comet designation (where number=2015, type=X, and name=N77)
|
Thank you for your contribution to sbpy, an Astropy affiliated package! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #422 +/- ##
==========================================
+ Coverage 84.61% 84.63% +0.01%
==========================================
Files 92 92
Lines 8420 8426 +6
==========================================
+ Hits 7125 7131 +6
Misses 1295 1295 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
added test invalid input
|
So the problem in issue #421 seems to actually be due to the second part of the asteroid designation also being a potential comet type, so parse_comet will interpret "2015XN77", "2015PN77", and "2015CN77" all as comets (broken down as number=2015, type=X/P/C, and name=N77), while "2015TN77" or any other letters in that first spot besides PDCX will (correctly) throw an error. The simplest fix seems to be to apply a validation test to the comet name to disallow comet names with numbers in the second position (but allow other non-alphabetic characters so names like d'Arrest don't also get screened out). I considered adding a validation test earlier on in the parsing to look for years followed by asteroid designations without spaces, but I feel like that has a better chance of introducing unintended consequences (especially once LSST takes us into the four-digit comet number era), whereas simply validating the name seems to address this specific issue without impacting any other functionality. |
…as packed asteroid desigs Fixed additional bug where a provisional comet designation with no forward slash and no spaces (e.g., "P2015XN77") was being interpreted as a packed asteroid designation (using only the "P2015" portion of the designation, which is technically unpackable into "252015"), but now raises a TargetNameParseError with the addition of a check to make sure a detected packed designation is not just a portion of a longer input string.
|
Thanks! |
Fixing a bug where parse_comet() incorrectly interprets input like "2015XN77" as a well-formed comet designation (where number=2015, type=X, and name=N77)