Skip to content

Bugfix: ALMA enhanced tables don't need qtable #3395

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

keflavich
Copy link
Contributor

This example results in an error:

from astroquery.alma import Alma
from astroquery import alma
from astropy.coordinates import SkyCoord
import astropy.units as u

query_results = Alma.query_region(SkyCoord.from_name('NGC 6334'), radius=0.2*u.deg)

etbl = alma.get_enhanced_table(query_results)

error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[1], line 8
      4 import astropy.units as u
      6 query_results = Alma.query_region(SkyCoord.from_name('NGC 6334'), radius=0.2*u.deg)
----> 8 etbl = alma.get_enhanced_table(query_results)

File [/red/adamginsburg/repos/astroquery/astroquery/alma/core.py:296](http://localhost:23456/red/adamginsburg/repos/astroquery/astroquery/alma/core.py#line=295), in get_enhanced_table(result)
    294     else:
    295         return _get_region(s_region.split())
--> 296 prep_table = result.to_qtable()
    297 s_region_parser = None
    298 for field in result.resultstable.fields:

AttributeError: 'Table' object has no attribute 'to_qtable'

This PR fixes the problem by skipping the to_qtable conversion.

However, the test is failing:

FAILED astroquery/alma/tests/test_alma.py::test_enhanced_table - TypeError: 'Record' object does not support item assignment

so I've not clearly solved the problem.

@andamian can you chime in?

@bsipocz
Copy link
Member

bsipocz commented Aug 14, 2025

Wow, how does this not raise an error, is it not covered by tests?

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.

Just guessing around what the problems might be

@@ -220,7 +220,7 @@ def get_enhanced_table(result):
"Please refer to https://astropy-regions.readthedocs.io/en/latest/installation.html for how to install it.")
raise

def _parse_stcs_string(input):
def s_region_parser(input):
Copy link
Member

Choose a reason for hiding this comment

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

I would leave this as a private method

return prep_table

for row in result:
row['s_region'] = s_region_parser(row['s_region'])
Copy link
Member

Choose a reason for hiding this comment

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

don't reuse/call the method inside its body

@@ -293,19 +293,10 @@ def _get_region(tokens):
return result
else:
return _get_region(s_region.split())
prep_table = result.to_qtable()
Copy link
Member

Choose a reason for hiding this comment

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

I'm not at all sure why this stopped working, we haven't really switch the API (as we haven't really switched out the TAP results to return Tables rather than DAL result tables as suggested #3244)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants