-
-
Notifications
You must be signed in to change notification settings - Fork 429
Adding query_all() to heasarc. #3499
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
base: main
Are you sure you want to change the base?
Conversation
|
@zoghbi-a I'm adding the extra angular offset column you requested. Each row in the result is a count of matching rows for that table, and I have added a distance computation into the query. But that's the distance for every matching row, and so we need to choose an aggregate in the SQL that makes the most sense here, like MAX() or AVG(). MAX makes sense when you do not specify a radius, because that means it's using a different radius for each catalog and you otherwise don't see any indication of that in the results. If you do specify a radius, then I'm not sure there's anything useful to aggregate about the angular offset. Did you have a use case in mind that helps? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3499 +/- ##
==========================================
+ Coverage 71.95% 71.97% +0.01%
==========================================
Files 235 235
Lines 20328 20403 +75
==========================================
+ Hits 14627 14685 +58
- Misses 5701 5718 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…roquery into tj-heasarc-query_all-rebased
Rebased and squashed after the #3493 fiasco, which we can discard.
This includes comments made there, but the changes were confused so it needs review again.