Satip icon indicating copy to clipboard operation
Satip copied to clipboard

Fix PR data test plotting script

Open jacobbieker opened this issue 11 months ago • 14 comments

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.

jacobbieker avatar Mar 06 '24 14:03 jacobbieker

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 avatar Mar 06 '24 17:03 vikasgrewal16

@jacobbieker can I be assign to this?

norbline avatar Mar 07 '24 08:03 norbline

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.

norbline avatar Mar 07 '24 08:03 norbline

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 avatar Mar 07 '24 17:03 vikasgrewal16

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

vikasgrewal16 avatar Mar 12 '24 09:03 vikasgrewal16

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.

jacobbieker avatar Mar 12 '24 10:03 jacobbieker

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

vikasgrewal16 avatar Mar 13 '24 11:03 vikasgrewal16

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.

jacobbieker avatar Mar 13 '24 12:03 jacobbieker

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

terapyo1304 avatar Mar 21 '24 14:03 terapyo1304

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

jacobbieker avatar Mar 21 '24 14:03 jacobbieker

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

vikasgrewal16 avatar Mar 22 '24 22:03 vikasgrewal16

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!

jacobbieker avatar Mar 23 '24 08:03 jacobbieker

works. I'm on it.

terapyo1304 avatar Mar 25 '24 13:03 terapyo1304

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?

suleman1412 avatar Apr 18 '24 13:04 suleman1412