openclimatefix / Satip

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

Fix PR data test plotting script #231

Closed jacobbieker closed 5 months ago

jacobbieker commented 6 months ago

Describe the bug

The data plotting scripts located here: scripts/generate_test_plots.py are out of date and do not work anymore. They are very helpful for auto plotting data from the library on all PRs, checking if the data processing is still working right visually.

Expected behavior

On PRs, the bot should add the plots automatically.

Additional context

There is also a GitHub Action that would need to be updated.

vikasgrewal16 commented 6 months ago

i have checked that the test plots are not generating because these files do not exist anymore on running python scripts/generate_test_plots.py i have come up with an error as: python: can't open file '/home/grewal/Satip/scripts/generate_tests_plots.py': [Errno 2] No such file or directory

now can u suggest me what should be done

norbline commented 6 months ago

@jacobbieker can I be assign to this?

norbline commented 6 months ago

i have checked that the test plots are not generating because these files do not exist anymore on running python scripts/generate_test_plots.py i have come up with an error as: python: can't open file '/home/grewal/Satip/scripts/generate_tests_plots.py': [Errno 2] No such file or directory

now can u suggest me what should be done

@vikasgrewal16 I think that is not the issue,the error message you encountered indicates that the file you are trying to open does not exist in the specified directory.

vikasgrewal16 commented 6 months ago

i have checked that the test plots are not generating because these files do not exist anymore on running python scripts/generate_test_plots.py i have come up with an error as: python: can't open file '/home/grewal/Satip/scripts/generate_tests_plots.py': [Errno 2] No such file or directory now can u suggest me what should be done

@vikasgrewal16 I think that is not the issue,the error message you encountered indicates that the file you are trying to open does not exist in the specified directory.

I know this is not the issue that's why i mentioned that files do not exist but I wanted to ask that what have to be done now what should be the next step now.

vikasgrewal16 commented 6 months ago

can i get your review on this issue @jacobbieker ? what exacttly needed to be done and can you please guide me to that

jacobbieker commented 6 months ago

Hi, the file is here: https://github.com/openclimatefix/Satip/blob/main/scripts/generate_test_plots.py there is an extra s on the path you were using, which is why that file didn't exist. What needs to be done is update that script to use the newer functions in Satip to load and plot the data. It should still be somewhat close to correct. Then a new workflow under .github/workflows/ will need to be added that calls it. The old one was this:

name: Plots

on: [push]

jobs:
  plots:
    strategy:
      matrix:
        python-version: ["3.9"]
        os: ["ubuntu-latest"]
    runs-on: ${{ matrix.os }}
    steps:
      - uses: actions/checkout@v2
      - uses: iterative/setup-cml@v1
      - name: Set up Python ${{ matrix.python-version }}
        uses: actions/setup-python@v2
        with:
          python-version: ${{ matrix.python-version }}
      - name: Do some macOS specific installs for Python 3.9
        run: |
          brew install ${{inputs.brew_install}}
        if: matrix.os == 'macos-latest'
      - name: Do some Ubunutu specific installs for Python 3.9
        if: runner.os == 'Linux'
        run: |
          sudo apt install ${{inputs.sudo_apt_install}}
      - name: Install dependencies
        run: |
          # $CONDA is an environment variable pointing to the root of the miniconda directory
          $CONDA/bin/conda install eccodes numpy matplotlib rasterio satpy[all] cartopy -c conda-forge
          $CONDA/bin/pip install -e .
      - name: Run plotting script
        env:
          REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        run: |
          export EUMETSAT_USER_KEY="${{ secrets.EUMETSAT_USER_KEY }}"
          export EUMETSAT_USER_SECRET="${{ secrets.EUMETSAT_USER_SECRET }}"
          $CONDA/bin/python scripts/generate_test_plots.py
          cml-publish cloud_mask_UK.png --md >> report.md
          cml-publish rss_UK.png --md >> report.md
          cml-publish hrv_UK.png --md >> report.md
          cml-publish cloud_mask_RSS.png --md >> report.md
          cml-publish rss_RSS.png --md >> report.md
          cml-publish hrv_RSS.png --md >> report.md
          # cml-publish tailored_cloud_mask.png --md >> report.md
          # cml-publish tailored_rss.png --md >> report.md
          cml-send-comment report.md
      - name: Archive plots
        uses: actions/upload-artifact@v2
        with:
          name: plots
          path: |
            *.png

But that will need to be updated to the latest versions of Python, the CML upload artifact, etc.

vikasgrewal16 commented 6 months ago

i have added a new file which includes workflow and i have encountered with an error please can you guide mt through this? @jacobbieker image

jacobbieker commented 6 months ago

Hi, that is in the script, its one of the changes that has happened since the plotting script was used last. Most of these errors should point you to what needs to change in it.

terapyo1304 commented 6 months ago

Hey Jacob, Aryaman this side. Can this issue be assigned to me?

jacobbieker commented 6 months ago

Hi! Possibly, @vikasgrewal16 are you still working on this?

vikasgrewal16 commented 6 months ago

Hi! @jacobbieker Due to my college exams I wasn't able to contribute but from now onwards i will be actively participating in all these contributions. Sorry to inform you late Regards

jacobbieker commented 6 months ago

Okay, sounds good! @terapyo1304 if you are still interested in this, one thing that would be helpful is adding plotting of IODC imagery from EUMETSAT. The current script was built just to plot around the UK and Europe, but now that we are using Satip for Indian Ocean imagery as well, it would be nice to have that added too!

terapyo1304 commented 6 months ago

works. I'm on it.

suleman1412 commented 5 months ago

hi @jacobbieker , I was looking into this and tried running python3 generate_test_plots.py which downloaded the latest eumetsat files. However, it gave a ValueError, and I'm assuming this is the issue that is causing the problems?