[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 74.05%. Comparing base (c41a46a) to head (274d877).
⚠️ Report is 36 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3673      +/-   ##
==========================================
+ Coverage   73.86%   74.05%   +0.19%     
==========================================
  Files         228      228              
  Lines       21475    21649     +174     
==========================================
+ Hits        15863    16033     +170     
- Misses       5612     5616       +4     

☔ 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?

[bsipocz]

The API looks reasonable, my only comment is about if we really need that type annotation.

[bsipocz]

I'm not at all a fan of this, or in fact any typing -- we don't enforce it and we are never dispatching into a computationally performant algorithm either.
Is there a particular reason you want this?

[cosmoJFH]

I'm not at all a fan of this, or in fact any typing -- we don't enforce it and we are never dispatching into a computationally performant algorithm either. Is there a particular reason you want this?

Hi @bsipocz, we just thought it would be a useful way to distinguish between the three possible scenarios while maintaining backward compatibility. What would you suggest?

[bsipocz]

ahh, ok, I see what you mean. I suppose it's OK then for this one case, but overall I would still say that we may not want to overcomplicate our life by doing types when this is not code where we ever really need.

[cosmoJFH]

Hi @bsipocz, we’d be happy to implement whichever solution you think is best. The current implementation is one possible approach, but we’re open to your suggestions and would be happy to follow your recommendation.

[bsipocz]

The RTD failure is genuine, and to fix it I added commit here that turns off typehints for the documentation.

That being said, we'll keep seeing RTD failures, but those are unrelated and are due to the currently unavailable requests intersphinx mapping.