ASB-30568: Adding read_product Function to load files from s3 to memory#3561
ASB-30568: Adding read_product Function to load files from s3 to memory#3561AlexReedy wants to merge 28 commits intoastropy:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3561 +/- ##
==========================================
- Coverage 73.22% 73.14% -0.09%
==========================================
Files 226 226
Lines 21000 21037 +37
==========================================
+ Hits 15378 15387 +9
- Misses 5622 5650 +28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bsipocz
left a comment
bsipocz
left a comment
There was a problem hiding this comment.
The commit history is a bit all over the place. Could you please clean it up and squash to logical chucks rather than quasi random back and forth? Thanks!
I suppose this will also need narrative documentation; but I'll leave the more detailed review to @snbianco
| except Exception as e: | ||
| log.exception(f"Failed to open ASD File: {product_path} {e}") | ||
| else: | ||
| print("Unsupported extension type") |
There was a problem hiding this comment.
please don't print anything; either raise proper warning or error classes or put this behind a verbose option.
There was a problem hiding this comment.
ah thank you! I thought I got rid of all of those
| '`~astroquery.mast.ObservationsClass.enable_cloud_dataset` method.' | ||
| ) | ||
|
|
||
| asdf_packages = ["asdf", "s3fs", "fsspec", "lz4", "gwcs"] |
There was a problem hiding this comment.
does all of these really need to be checked here even as they are not directly been used? If asdf requires the whole list then these checks should be dealt with upstream in asdf itself.
There was a problem hiding this comment.
This has been an ongoing question with the implementation. Required would be asdf, s3fs, fsspec for the function to work. for asdf specifically, lz4 seems to be the primary compression algorithm being used for asdf and gwcs is for the general.
I know that gwcs is in their test environment, and they also make calls to lz4 but I don't believe it's included when it's installed. I agree it should probably be upstream in asdf.
2 participants
Adding in ability to read FITS and ASDF data products to memory from s3:// using Observations.read_product() function