Skip to content
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

fix: use left join when adding tables instead of inner join #3199

Merged

Conversation

ManonMarchand
Copy link
Member

@ManonMarchand ManonMarchand commented Feb 3, 2025

This fixes #3197

The behavior that people expect is not the one that we get with the default inner join:

from astroquery.simbad import Simbad
simbad = Simbad()
simbad.add_votable_fields("mesdistance")
simbad.query_object("vega")

with inner join

No measurement --> no result

WARNING: NoResultsWarning: The request executed correctly, but there was no data corresponding to these criteria in SIMBAD [astroquery.simbad.core]
<Table length=0>
main_id    ra     dec   coo_err_maj coo_err_min coo_err_angle coo_wavelength coo_bibcode mesdistance.bibcode ... mesdistance.mespos mesdistance.method mesdistance.minus_err mesdistance.minus_err_prec mesdistance.plus_err mesdistance.plus_err_prec mesdistance.qual mesdistance.unit matched_id
          deg     deg       mas         mas          deg                                                     ...                                                                                                                                                                                   
 object float64 float64   float32     float32       int16          str1         object          object       ...       int16              object              float64                  int16                  float64                  int16                 str1            object        object  
------- ------- ------- ----------- ----------- ------------- -------------- ----------- ------------------- ... ------------------ ------------------ --------------------- -------------------------- -------------------- ------------------------- ---------------- ---------------- ----------

with left join

No measurement --> masked data

<Table length=1>
 main_id         ra              dec       coo_err_maj coo_err_min coo_err_angle coo_wavelength     coo_bibcode     mesdistance.bibcode ... mesdistance.method mesdistance.minus_err mesdistance.minus_err_prec mesdistance.plus_err mesdistance.plus_err_prec mesdistance.qual mesdistance.unit matched_id
                deg              deg           mas         mas          deg                                                             ...                                                                                                                                                                
  object      float64          float64       float32     float32       int16          str1             object              object       ...       object              float64                  int16                  float64                  int16                 str1            object        object  
--------- ---------------- --------------- ----------- ----------- ------------- -------------- ------------------- ------------------- ... ------------------ --------------------- -------------------------- -------------------- ------------------------- ---------------- ---------------- ----------
* alf Lyr 279.234734787025 38.783688956244     3.51118     2.81205            90              O 2007A&A...474..653V                     ...                                       --                         --                   --                        --                                    NAME Vega

Other changes

This PR also adds a new section in the documentation that tries to explain measurement tables in SIMBAD.

Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.34%. Comparing base (6b0af57) to head (3e9ff61).
Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3199   +/-   ##
=======================================
  Coverage   68.34%   68.34%           
=======================================
  Files         231      231           
  Lines       19190    19190           
=======================================
  Hits        13116    13116           
  Misses       6074     6074           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@keflavich keflavich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good technically. I have a question about how one figures out the parameter names.

docs/simbad/simbad.rst Outdated Show resolved Hide resolved
docs/simbad/simbad.rst Show resolved Hide resolved
@ManonMarchand ManonMarchand marked this pull request as draft February 3, 2025 12:53
@ManonMarchand ManonMarchand force-pushed the fix-use-left-join-in-measurement-tables branch from d2d944b to 3e9ff61 Compare February 3, 2025 13:31
@ManonMarchand ManonMarchand marked this pull request as ready for review February 3, 2025 13:32
@bsipocz bsipocz added this to the v0.4.10 milestone Feb 3, 2025
Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bsipocz bsipocz merged commit e2db773 into astropy:main Feb 3, 2025
12 checks passed
@ManonMarchand ManonMarchand deleted the fix-use-left-join-in-measurement-tables branch February 4, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Measurement tables should be joined with a left join in simbad
3 participants