[WIP] New files for putting icepyx examples on pangeo-gallery #125

salvis2 · 2020-08-18T21:27:32Z

This is the start of the work that will get some icepyx example notebooks onto Pangeo Gallery. @JessicaS11 and I have had some conversation about what this will look like and we are now putting the conversation on GitHub.

This building of notebooks can be another entry point / set of learning resources for users new to Pangeo and ICESat-2. Some of this work may be relevant later for the icepyx gallery that would be on ReadTheDocs; binderbot can maybe be repurposed for that. Currently, we still need to choose / work on:

a binderhub to build notebooks on. I've put mybinder.org as the default.
a docker image to use as the computing environment for building the notebooks. Dockerfile is currently blank.
an updated description in binder-gallery.yaml (if desired)
what notebooks to build
a mechanism for naming the notebooks that we want built (because we do not want all of the notebooks built). This should look like a wildcard-style pattern for file names and will replace paths: - '**.ipynb' in .github/workflows/binderbot.yaml.
how to reference the desired thumbnail image in a way that isn't thumbnail.png at the root of the repo
Adding this repo as a submodule to the Pangeo Gallery. I will set this up near the end via the instructions present in Pangeo Gallery's Contributor Guide.

This new PR is attempting to fix my git issues by having a specific feature branch...

Should be using reg_a.granules.avail instead of reg_a.granules

Reformat CONTRIBUTORS.rst, add myself, fix typo

Updated Contributers, added my name

Codecov

Tj dev

update documentation and readme with clearer install instructions (including for windows users)

Update codecov badge to point to 'development' branch

Fix test failures in test_granules.py

mac OS upgrades can lead to machines unable to get IP from hostname. this is a fix to revert to using 'localhost' in case of a gaierror from socket.gethostbyname

…py#96) * add black pre-commit hook and reformat files using black * add github action workflow with flake8 on PRs

JessicaS11 · 2020-08-19T21:09:19Z

Okay, should I add the development branch to the GitHub Actions (which only has master right now) or have only the development branch there?

If I understand it right, everything is controlled by the GitHub Actions, which are triggered when there are pushes/PRs to the specified branches. It makes sense to rebuild the notebooks when there is a new release (e.g. a push/PR/release on master) AND when there is a change to the gallery files that is merged (via PR) into development (for testing purposes). Right now it only does the former and we're looking to set it up to also do the latter, with the trigger either by changes to certain directories or a commit message code.

salvis2 · 2020-08-19T21:50:48Z

It makes sense to rebuild the notebooks when there is a new release (e.g. a push/PR/release on master) AND when there is a change to the gallery files that is merged (via PR) into development (for testing purposes). Right now it only does the former and we're looking to set it up to also do the latter, with the trigger either by changes to certain directories or a commit message code.

I agree. I think the main problem to solve then is where do you put the content in the latter case? I think as written, the action will build notebooks and put them on the binderbot_built branch. I don't think we would want test commits to development to overwrite / break the master rendered notebooks. Could maybe make another GitHub Action that skips the commit / push steps to just verify that the notebooks build when changes are made to the relevant files on development.

As far as changes to certain directories, the GitHub Action currently looks at:

paths:
  - 'examples/*.ipynb'
  - 'binder-gallery.yaml'
  - 'doc/source/_static/icepyx_logo.png'

JessicaS11 · 2020-08-26T18:41:36Z

I don't think we would want test commits to development to overwrite / break the master rendered notebooks. Could maybe make another GitHub Action that skips the commit / push steps to just verify that the notebooks build when changes are made to the relevant files on development

I agree completely. Doing a test build for things going into development seems like a good approach to me, provided it can provide more useful output than "failed" in the case of failure (e.g. the error message and traceback and failed notebook). Would it make sense to add this as part of the test suite (currently done with Travis CI)? It tests both the branch and the PR merge.

salvis2 · 2020-08-26T19:18:29Z

I agree completely. Doing a test build for things going into development seems like a good approach to me, provided it can provide more useful output than "failed" in the case of failure (e.g. the error message and traceback and failed notebook).

Yeah, unsure what can be done there. I can dig into that a bit.

Would it make sense to add this as part of the test suite (currently done with Travis CI)? It tests both the branch and the PR merge.

I have never really messed with Travis CI, I could probably do it with GitHub Actions though. If you know a bit about Travis, does it seem viable to take the Action in binderbot.yaml and get it in Travis?

salvis2 · 2020-08-26T19:25:48Z

Oh, I think it does provide what you are saying. Check out this log from the binderbot action on the Landsat-8 Gallery and let me know if that seems good enough.

weiji14 · 2020-08-26T22:11:12Z

.github/workflows/binderbot.yaml

+      - name: Commit files
+        if: github.ref == 'refs/heads/master'
+        run: |
+          git config --local user.email "[email protected]"
+          git config --local user.name "GitHub Action"
+          git add binder-gallery.yaml doc/source/_static/icepyx_logo.png examples/*.ipynb
+          git commit -m "Binderbot output"
+      - name: Push commit
+        if: github.ref == 'refs/heads/master'
+        uses: ad-m/github-push-action@master
+        with:
+          github_token: ${{ secrets.GITHUB_TOKEN }}
+          branch: ${{ steps.parse.outputs.binderbot_target_branch }}
+          force: true
+      - name: Trigger repository dispatch on Pangeo Gallery
+        if: github.ref == 'refs/heads/master'
+        run: curl -f https://pangeo-gallery-bot.herokuapp.com/gallery/submodule-dispatch/${{ github.repository }}


With the if: github.ref == 'refs/heads/master' line(s), it seems that the commit-push-deploy will only happen on the master branch (which is good). Would it make sense then to run this binderbot GIthub Action on Pull Requests targeting both the master and development branches then, so we can test that it works on development too? (See suggestion on other comment).

Would it make sense to add this as part of the test suite (currently done with Travis CI)? It tests both the branch and the PR merge.

I have never really messed with Travis CI, I could probably do it with GitHub Actions though. If you know a bit about Travis, does it seem viable to take the Action in binderbot.yaml and get it in Travis?

It should be fine having both Travis CI and Github Actions CI, a rewrite won't be needed. Travis will run the icepyx unit tests as usual and Github Actions will 'test' that the example gallery can be built correctly.

Aha! I think you are right @weiji14 . It should only run those last three on master, so we might have everything we need if we just add development to the branches for the Action to listen to.

.github/workflows/binderbot.yaml

Co-authored-by: Wei Ji <[email protected]>

salvis2 · 2020-08-26T23:07:40Z

And now binderbot is doing an Action! Nice.

salvis2 · 2020-08-26T23:40:16Z

Oh. I think the problem is that the only record of the Dockerfile is on my enable-gallery branch. binder-gallery.yaml is told to look on the master branch for the image information and doesn't find anything, so it gets the error:

ModuleNotFoundError: No module named 'xarray'

Seems like another thing that won't be properly tested until the merge happens.

weiji14 · 2020-08-27T00:07:30Z

.github/workflows/binderbot.yaml

+          gallery_repository: gallery_repository
+          binder_url: binder_url
+          binder_repo: binder_repo
+          binder_ref: binder_ref


Oh. I think the problem is that the only record of the Dockerfile is on my enable-gallery branch. binder-gallery.yaml is told to look on the master branch for the image information and doesn't find anything, so it gets the error:

ModuleNotFoundError: No module named 'xarray'

Seems like another thing that won't be properly tested until the merge happens.

Would it be possible to set the binder_ref to just point to the branch it is building on? E.g. development in this case.

Might be good to share a link to the pangeo-gallery documentation somewhere, so that others will know how this can be configured in the future.

Yeah, I can change that for a test to see if it builds correctly.

Not sure if it will help, but you can try to run Github Actions locally using https://github.com/nektos/act. Might be easier than just trial and error pushes to this Pull Request.

salvis2 · 2020-08-27T00:10:26Z

Per @weiji14 's suggestion, here is the Pangeo Gallery Contributor Guide that I have followed for this work.

Pointing to my fork and looking at the enable-gallery branch. Will remove later.

salvis2 · 2020-08-27T16:53:48Z

@weiji14 Hopefully the emails aren't too much. I think I will try pushing to my fork's master branch until it works. I'm not supremely familiar with binderbot; I've only used it to set up galleries from master branches of repos that are just gallery entries of other work. The errors listed are rather unfamiliar.

salvis2 · 2020-08-27T21:42:04Z

I think I've narrowed down the issue of 500 errors to something to do with the notebooks being in not the root directory. I've raised an issue on binderbot to investigate: pangeo-gallery/binderbot#32. This PR may have to wait until I figure that out.

weiji14 · 2020-08-27T21:48:02Z

Thanks for tracking down the issue! I was struggling to understand the error messages too. Just as an idea, would it be better (or easier) if we move the examples to a separate icepyx-examples repository under the same icepyx umbrella?

salvis2 · 2020-08-28T00:21:09Z

I think most of the gallery repos are just that: standalone repos with content showcasing how to use something from a different repo. I do think it would be much easier to set that up, but it is a separate thing to maintain. I'm more than happy to help with that if y'all want to go that route.

JessicaS11 · 2020-08-28T13:46:57Z

Thanks for all your conversation and troubleshooting, @salvis2 and @weiji14! It looks like this is shaping up quite well. As you've no doubt seen, we've already got a mix of GitHub actions and Travis CI, which I think is fine.

would it be better (or easier) if we move the examples to a separate icepyx-examples repository under the same icepyx umbrella?

I'm not opposed to this idea, and the icesat2py organization is meant for this type of thing. Now that icepyx is available to install with pip I think that alleviates many issues you might have with the code and examples in different repos. My only concern about separating them is whether or not it will make it feel more cumbersome (or simply be confusing) to new contributors to have the code and examples in different repos. Since git has such a steep learning curve, I'm hesitant to introduce any potential barriers to contributions by scientists who aren't trained in software development and don't have the time to invest in learning it.

JessicaS11 · 2020-08-28T13:50:19Z

binder/Dockerfile

@@ -0,0 +1 @@
+FROM uwhackweeks/icesat2:2020.06.15


With the most recent updates to icepyx, if we try to build against this Hackweek Docker image the examples will fail, since the icepyx example notebooks have been updated to use Query objects instead of Icesat2Data objects.

salvis2 · 2020-10-22T18:44:54Z

Closing in favor of setting up https://github.com/icesat2py/icepyx-gallery

weiji14 and others added 30 commits June 17, 2020 09:45

Let travis test everything in icepyx

94acb79

Fix test_granule_info

74dd0e3

Fix test_correct_granule_list_returned

929594e

Should be using reg_a.granules.avail instead of reg_a.granules

Reformat CONTRIBUTORS.rst, add myself, fix typo

8026be1

Test everything except behind_NSIDC_API_login.py

58ed1ea

add pr builds to travis

16f83e8

Merge branch 'development' into development

8da8239

Merge pull request icesat2py#76 from dshean/development

f4aee1d

Reformat CONTRIBUTORS.rst, add myself, fix typo

codeblock defined as none is not compatible with pypi markup

fcbe215

Added T Johnson to list of contributors

823cf34

Add codecov

aefbf6f

Updated Contributers, added my name

e948fa6

Add dev requirements and specify travis install

9a7eab6

Fix codecov badge links (use icesat2py repo)

b71d574

Updated installation instructions

35444ec

Changed installation instructions on readthedocs

96bab86

Merge pull request icesat2py#84 from icesat2py/anna_dev2

ca0b4fe

Updated Contributers, added my name

Merge branch 'development' into codecov

7cd1795

Merge pull request icesat2py#88 from wallinb/codecov

7bf566c

Codecov

Merge branch 'development' into tj_dev

9f25d59

Merge pull request icesat2py#85 from icesat2py/tj_dev

67619f8

Tj dev

Update codecov badge to point to 'development' branch

9809159

Merge branch 'development' into tj_docs

709cbd0

Merge pull request icesat2py#89 from icesat2py/tj_docs

62ce481

update documentation and readme with clearer install instructions (including for windows users)

Merge branch 'development' into readme-fix

16466f8

Merge pull request icesat2py#90 from icesat2py/readme-fix

98e68fa

Update codecov badge to point to 'development' branch

Merge branch 'development' into travis_test_all

9f5e8a4

Merge pull request icesat2py#61 from icesat2py/travis_test_all

82f4aec

Fix test failures in test_granules.py

add try/except for gaierror to address icesat2py#97 (icesat2py#98)

f9d96d1

mac OS upgrades can lead to machines unable to get IP from hostname. this is a fix to revert to using 'localhost' in case of a gaierror from socket.gethostbyname

add Black (pre-commit hook) and flake8 (PR annotations) runs (icesat2…

6f8103f

…py#96) * add black pre-commit hook and reformat files using black * add github action workflow with flake8 on PRs

salvis2 added 3 commits August 19, 2020 11:24

Update paths to only build notebooks in the examples/ folder

9067503

Attempt to use a thumbnail that isn't in the root directory

fe57648

Use latest ICESat-2 Hackweek image for now

975ae6c

salvis2 force-pushed the enable-gallery branch from 35e543a to 975ae6c Compare August 19, 2020 18:25

weiji14 reviewed Aug 26, 2020

View reviewed changes

salvis2 and others added 2 commits August 26, 2020 16:04

Enable the Action of the development branch

f4dc945

Co-authored-by: Wei Ji <[email protected]>

Enable dev branch for pushes too

96a6fc3

Add newline

5427476

weiji14 reviewed Aug 27, 2020

View reviewed changes

salvis2 added 3 commits August 26, 2020 17:11

Try looking at the development branch to get a successful test build

b2d44f3

Second test of build

d5ef772

Pointing to my fork and looking at the enable-gallery branch. Will remove later.

Newline to pass linting action

88404c9

JessicaS11 reviewed Aug 28, 2020

View reviewed changes

JessicaS11 force-pushed the development branch from cb9fa86 to e7a9af0 Compare September 14, 2020 14:02

salvis2 mentioned this pull request Oct 13, 2020

Set up the icepyx gallery icesat2py/icepyx-gallery#1

Open

salvis2 closed this Oct 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] New files for putting icepyx examples on pangeo-gallery #125

[WIP] New files for putting icepyx examples on pangeo-gallery #125

salvis2 commented Aug 18, 2020 •

edited

Loading

JessicaS11 commented Aug 19, 2020

salvis2 commented Aug 19, 2020

JessicaS11 commented Aug 26, 2020

salvis2 commented Aug 26, 2020

salvis2 commented Aug 26, 2020

weiji14 Aug 26, 2020

salvis2 Aug 26, 2020

salvis2 commented Aug 26, 2020

salvis2 commented Aug 26, 2020

weiji14 Aug 27, 2020

salvis2 Aug 27, 2020

weiji14 Aug 27, 2020

salvis2 commented Aug 27, 2020

salvis2 commented Aug 27, 2020

salvis2 commented Aug 27, 2020

weiji14 commented Aug 27, 2020 •

edited

Loading

salvis2 commented Aug 28, 2020

JessicaS11 commented Aug 28, 2020

JessicaS11 Aug 28, 2020

salvis2 commented Oct 22, 2020

[WIP] New files for putting icepyx examples on pangeo-gallery #125

[WIP] New files for putting icepyx examples on pangeo-gallery #125

Conversation

salvis2 commented Aug 18, 2020 • edited Loading

JessicaS11 commented Aug 19, 2020

salvis2 commented Aug 19, 2020

JessicaS11 commented Aug 26, 2020

salvis2 commented Aug 26, 2020

salvis2 commented Aug 26, 2020

weiji14 Aug 26, 2020

Choose a reason for hiding this comment

salvis2 Aug 26, 2020

Choose a reason for hiding this comment

salvis2 commented Aug 26, 2020

salvis2 commented Aug 26, 2020

weiji14 Aug 27, 2020

Choose a reason for hiding this comment

salvis2 Aug 27, 2020

Choose a reason for hiding this comment

weiji14 Aug 27, 2020

Choose a reason for hiding this comment

salvis2 commented Aug 27, 2020

salvis2 commented Aug 27, 2020

salvis2 commented Aug 27, 2020

weiji14 commented Aug 27, 2020 • edited Loading

salvis2 commented Aug 28, 2020

JessicaS11 commented Aug 28, 2020

JessicaS11 Aug 28, 2020

Choose a reason for hiding this comment

salvis2 commented Oct 22, 2020

salvis2 commented Aug 18, 2020 •

edited

Loading

weiji14 commented Aug 27, 2020 •

edited

Loading