Add galactic_centre() Source template (Gillessen+2017 orbits)#180
Merged
Conversation
Builds a scopesim.Source for the Sgr A* star field at a user-specified epoch. Vendors the Gillessen et al. 2017 orbital catalogue (CDS J/ApJ/837/30) and its evaluation code from the standalone `gillessen` script collection. Each star gets its own RV-shifted spectrum (B0V for early-type, M8III for late-type by default), scaled to its K-band magnitude through Paranal/NACO.Ks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
astronomyk
added
enhancement
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #180 +/- ##
==========================================
+ Coverage 75.41% 77.09% +1.67%
==========================================
Files 32 34 +2
Lines 1562 1698 +136
==========================================
+ Hits 1178 1309 +131
- Misses 384 389 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 tasks
teutoburg
approved these changes
May 14, 2026
teutoburg
left a comment
teutoburg
left a comment
Contributor
There was a problem hiding this comment.
Now, I could ask you to use Astropy SkyCoord for some of this, or be more consistent with where units are and aren't used, or to just use units instead of bothering with np.deg2rad, or complain that some of what Claude gave you there is rather YAGNI, or even whine about style. But this is Templates, aka the dumpster. Who cares if the dumpster is on fire, as long as it burn bright enough to convince someone to continue funding us 🙂
7 tasks
Contributor
Author
|
Indeed. I agree with all of those statements. I might see what claude responds to your comment. Maybe it takes a long hard look at itself =) |
Contributor
|
Don't bother wasting tokens there... |
The `library` parameter only ever raised a ValueError for any non-default value, and `catalogue` / `R0` were passthroughs that nobody would ever override. Removing all three. Other items teutoburg flagged but not changed here: - SpT warn-and-default branch stays: the Gillessen catalogue legitimately has empty SpT cells for S39 and S55, so this isn't defensive paranoia. - Unit-boundary round-trip (the rv strip-and-reattach) is a thing on downstream branches that add an rv column to the fields table, not on this branch alone. Will be addressed when integrating with globular_cluster_imbh. - SkyCoord / np.deg2rad style preferences: declined. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
astronomyk
added a commit
that referenced
this pull request
May 14, 2026
The strip-and-reattach round-trip for the rv column was introduced when the rv column was added on the animation branch. Now that cluster_plots has fast-forwarded into globular_cluster_imbh, fix it here: - galactic_centre.py and globular_cluster.py hold rv as a Quantity from the moment it is computed (or pulled from the gillessen QTable) through to the final fields-table column. Only stripped to a plain float at the per-star Spextrum.redshift(vel=...) call site. SpT mapping refresh on galactic_centre.py: switched the if/elif/else to a dict.get() lookup; updated warning wording to "missing or unrecognised SpT" (the Gillessen catalogue legitimately has empty SpT cells for S39 and S55). Test regex updated to match. YAGNI cleanup from the original review (drop library/catalogue/R0 params) is already in main via PR #180; not re-applied here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds a scopesim.Source for the Sgr A* star field at a user-specified epoch. Vendors the Gillessen et al. 2017 orbital catalogue (CDS J/ApJ/837/30) and its evaluation code from the standalone
gillessenscript collection. Each star gets its own RV-shifted spectrum (B0V for early-type, M8III for late-type by default), scaled to its K-band magnitude through Paranal/NACO.Ks.