Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 69.08%. Comparing base (7494746) to head (fe2614f).
Report is 227 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3252      +/-   ##
==========================================
- Coverage   69.09%   69.08%   -0.01%     
==========================================
  Files         232      232              
  Lines       19637    19639       +2     
==========================================
  Hits        13568    13568              
- Misses       6069     6071       +2     

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

This looks good and it's the right thing to do.
My only concern is that we are breaking the backwards compatibility without warning (and we are not using type hints either to allow at least IDEs to issue warnings). What I did for the other queries in alma (and honestly I don't know how I've missed query_sia) was to add a flag enhanced_results to control the transition of the returned type. Right now it's optional but we can change it to issue a warning and eventually it can change the default to true and error when false. Note that getting rid of this optional attribute at the end also constitutes an API change but at least that it's more obvious.
Speaking of alma.query_sia - I would suggest using the same mechanism as the other alma query methods. In alma we actually go a bit further with the enhanced table and besides table we also try to make something useful of the s_region field. That unfortunately introduces an optional dependency (astropy.regions). If that feature is useful to irsa as well, this feature this could be a case for turning it into a mandatory dependency. I don't know how good of an idea this is @keflavich?

OK, that's all reasonable. My main concern is to return pyvo tables rather than if it's a Table or a QTable.

We also just very recently added query_sia and query_ssa to IRSA, so I can go ahead with PR as is and just mention the API change in the changelog. I think it will have way less user impact than waiting to do it in the next one.
As for TAP, again, my main concern is to have pyvo classes returned and then the user code looks like pyvotable.to_table().to_pandas() at multiple places in the code. They almost always don't need access to the pyvotable (I say almost always, as I'm sure they can be a corner case that I haven't yet come across, but in practice, I haven't yet seen a case where they needed it).

So, my question: can we merge this as is, or you rather have me just do IRSA for now and leave the alma part for the next release?

My impression is that alma is already used extensively and I'd rather take a gradual approach to upgrading it. Not great to have similar but different APIs but I don't want to inconvenience too many users. So please do just irsa for now. I will have to open a ticket and add the enhanced_table attribute to alma.query_sia.

OK, the alma part has been pulled out into #3261.