openclimatefix / Satip

Satip contains the code necessary for retrieving, transforming and storing EUMETSAT data
https://satip.readthedocs.io/
MIT License
41 stars 29 forks source link

Add GOES Data Download Manager Script #240

Closed 14Richa closed 3 months ago

14Richa commented 3 months ago

Pull Request

Description

This pull request adds a script for downloading GOES data. The script uses the GOES2GO library to download data for a specified time range and product. It includes functionality to save the downloaded data to a specified directory and log any errors encountered during the download process.

Fixes #222

Checklist:

14Richa commented 3 months ago

This is a good start! Thanks for this work. Its getting closer to what I was thinking for the image downloading. Mostly just some comments related to what was mentioned in #222, and some more simple checks to speed up the downloading and not download the same data multiple times.

Thanks for the comments @jacobbieker! I've addressed most of them and incorporated your suggestions. I've created a main DownloadManager class combining functionalities for EUMETSAT and GOES downloads. Also, I'm thinking of changing the filename from eumetsat.py to download_manager.py for better clarity. What do you think? Do you have any other suggestions for the filename?

jacobbieker commented 3 months ago

This is a good start! Thanks for this work. Its getting closer to what I was thinking for the image downloading. Mostly just some comments related to what was mentioned in #222, and some more simple checks to speed up the downloading and not download the same data multiple times.

Thanks for the comments @jacobbieker! I've addressed most of them and incorporated your suggestions. I've created a main DownloadManager class combining functionalities for EUMETSAT and GOES downloads. Also, I'm thinking of changing the filename from eumetsat.py to download_manager.py for better clarity. What do you think? Do you have any other suggestions for the filename?

Yeah, making it download_manager.py is better, and moving the EUMETSATDownloadManager to a eumetsat.py file.

14Richa commented 3 months ago

Nice improvements! This kind of design is good, especially for being able to add more providers in the future. There will need to be some more changes to the code, as DownloadManager now doesn't do what it did before. So if you could update the other calls to be the EUMETSATDownloadManager that would be great! And might be the last changes for this PR I think.

Thanks for the feedback @jacobbieker ! I have updated the code as per your suggestion, changing the other calls to use EUMETSATDownloadManager.

jacobbieker commented 3 months ago

Also, there will need to be a few updates to the tests, as they have changed in #229

14Richa commented 3 months ago

Hey @jacobbieker, both of my tests are running successfully. Can we merge this PR now?

peterdudfield commented 2 months ago

@all-contributors please add @14Richa for code

allcontributors[bot] commented 2 months ago

@peterdudfield

I've put up a pull request to add @14Richa! :tada: