Skip to content

Conversation

@eteq
Copy link
Member

@eteq eteq commented Dec 4, 2025

The main purpose of this PR is to add download_products_iter method to mast.Observations. My motivation here is to allow something like the following:

for whatever in tqdm(Observations.download_products_iter(product_list)):
    print(... something of interest using whatever...)

which then allows downloading a large number of files with both a progress bar and some custom progress reporting. Of course, verbose=True does something akin to this, but I find sometimes for large downloads I want more control of the progress reporting.

This is a draft at this stage, with no tests or doc updates because I figured it was better to do that before doing all the rest of the work in case this has been considered and rejected in the past. There might also be cleaner ways to split up the generator and the non-generator version, but this seemed the most low-impact way in case the private method is used in the wild elsewhere.

cc @snbianco I think is the right person?

@eteq eteq added the mast label Dec 4, 2025
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 27.58621% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.41%. Comparing base (4c01218) to head (0a3f04a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
astroquery/mast/observations.py 27.58% 21 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3475      +/-   ##
==========================================
- Coverage   71.48%   71.41%   -0.07%     
==========================================
  Files         234      234              
  Lines       20096    20121      +25     
==========================================
+ Hits        14365    14369       +4     
- Misses       5731     5752      +21     

☔ View full report in Codecov by Sentry.
📢 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.

@snbianco
Copy link
Contributor

snbianco commented Dec 4, 2025

Hey Eric, thanks for this PR! I do think this is worth doing as it provides more powers to users without breaking the API. Users often want to download a ton of data from us, and this solves a real unmet need that we have.

Since so much of download_products is duplicated in download_products_iter, I wondered if it might be better to add a parameter to download_products, something like return_iter. However, after thinking on it some more, that would muddy the API a bit, and I think the separated functions is the way to go. That being said, the duplicated logic should probably be pulled out into a helper function(s).

@keflavich
Copy link
Contributor

Agreed @snbianco, I like the idea of factoring out common code (e.g., filtering the selections) but keeping the API @eteq proposes

@bsipocz
Copy link
Member

bsipocz commented Dec 4, 2025

Agreed @snbianco, I like the idea of factoring out common code (e.g., filtering the selections) but keeping the API @eteq proposes

I'm not sure about the proposed API though, MAST is already such an odd ball out and this just makes it even more complicated.

We still working on removing function duplicates that are doing almost the same, and thus I would be careful recreating the same problem down the line for the download functions.
So, ultimately having a kwarg that makes download responses generators is something we could/would suggest to other modules and I would highly prefer that.

@bsipocz bsipocz added this to the 0.4.12 milestone Dec 4, 2025
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.

4 participants