Handle an s3 URI in asdf_cut() #117
Conversation
|
Double check if any of the unit tests need to be updated for this. |
| @@ -247,8 +248,15 @@ def asdf_cut(input_file: str, ra: float, dec: float, cutout_size: int = 20, | |||
| an image cutout object | |||
| """ | |||
|
|
|||
| # if file comes from AWS cloud bucket, get URL | |||
| file = input_file | |||
| if isinstance(input_file, str) and input_file.startswith('s3://'): | |||
There was a problem hiding this comment.
One thing we may want to do at some point is expand this to support pathlib.Path as input, in addition to a string. I don't think pathlib.Path handles s3 urls, but it looks like there are packages that do, e.g. s3path or s3pathlib. I don't think we need to do it for this PR, but maybe we can create an issue for the future.
There was a problem hiding this comment.
Cool, will make a new issue for this!
There was a problem hiding this comment.
Issue is ASB-27085
|
Trying to write unit tests for this now, and wondering what should be used for the |
|
@snbianco We probably don't need to test a real file. I think we can get away with a fake file for testing, or if there is a public ST S3 file to use for testing? If we turn lines 251-257 into a tiny function, we could test that functionality separately from any asdf stuff. That it returns the correct file or s3 file as input? That would satisfy me for a test. It looks like there are packages for mocking s3 aws services, like |
|
I generally recommend mocking external resources rather than relying on pings to public files. At a glance, it looked like your approach was good. Basically the goal is to confirm the functions were called with the parameters you expected; the exact logic of the s3fs functions is ideally something that package has verified already. |
If an s3 URI is passed in as the
input_fileparameter to theasdf_cutfunction, open the resource withs3fsand use its http URL inasdf.open().This prevents users from having to open the resource themselves before passing the file into the
asdf_cutfunction.