GAIA: update the method load_data to return the path of the downloaded DataLink archive

[cosmoJFH]

Dear Astroquery team,

we have opened this PR since we'd like to update the Gaia.load_data method to return the path of the downloaded DataLink archive when dump_to_file=True.

Currently, load_data prints the generated archive path but does not make it available programmatically. Returning the path allows users to directly access and process downloaded products, which is particularly useful for the upcoming Gaia DR4 residual_image products.

We have implemented the following changes:

1. Return None when dump_to_file=False.
2. Return the absolute path to the downloaded ZIP file when dump_to_file=True.
3. Preserve the existing method signature and backwards compatibility.

Example

dl_dict, out_path = Gaia.load_data(
    ids=ids,
    data_release="Gaia DR4_RC3",
    retrieval_type="RESIDUAL_IMAGE",
    data_structure="DATAMODEL_STANDARD",
    dump_to_file=True,
    format="fits",
)

cc @esdc-esac-esa-int
jira: GAIASA-3522

[keflavich]

The proposed API seems fine to me.

This should be accompanied by a warning, since it will change the output of load_data from a single table entry to (table, string), right?

An alternative approach is to leave the default behavior as-is (table = Gaia.load_data(dump_to_file=False)) and only add the new return if the file is dumped (table, path = Gaia.load_data(dump_to_File=True)).

I may slightly prefer the latter but I don't feel strongly. I'll check what we do elsewhere in astroquery...

[cosmoJFH]

The proposed API seems fine to me.

This should be accompanied by a warning, since it will change the output of load_data from a single table entry to (table, string), right?

An alternative approach is to leave the default behavior as-is (table = Gaia.load_data(dump_to_file=False)) and only add the new return if the file is dumped (table, path = Gaia.load_data(dump_to_File=True)).

I may slightly prefer the latter but I don't feel strongly. I'll check what we do elsewhere in astroquery...

Hi @keflavich, thanks for your feedback. I will discuss this alternative implementation with the rest of the team.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.80%. Comparing base (c777720) to head (2a1e2be).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3673      +/-   ##
==========================================
+ Coverage   73.76%   73.80%   +0.04%     
==========================================
  Files         230      230              
  Lines       21570    21579       +9     
==========================================
+ Hits        15911    15927      +16     
+ Misses       5659     5652       -7     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cosmoJFH]

We’d prioritize backward compatibility, so we’ve updated the implementation as follows:

• dump_to_File=False → dict[str, Table] (original behavior)
• dump_to_File=True → tuple[dict[str, Table], str]

We think this is the least disruptive migration, although it does make the type annotation a little more complicated.

We haven’t added a warning because there’s no deprecated behaviour for existing users.

[keflavich]

I'm happy with that change.

I would still want some kind of warning about the API change, but this can just go in the changelog, not necessarily the code - the return type may now be a tuple instead of a dict.

@bsipocz what do you think?