Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre commit #17

Merged
merged 16 commits into from
Aug 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions .github/workflows/black.yaml

This file was deleted.

32 changes: 32 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Continuous Integration
on:
push:
branches:
- "*"
pull_request:
branches:
- "*"

jobs:
build:
name: Test on ubuntu
runs-on: ubuntu-latest
strategy:
fail-fast: false
steps:
- uses: actions/checkout@v2
- name: Install conda
uses: goanpeca/setup-miniconda@v1
with:
auto-update-conda: true
activate-environment: hires-marbl
environment-file: environments/environment.yaml
auto-activate-base: false

- name: Show conda environment
shell: bash -l {0}
run: conda list

- name: Run Tests
shell: bash -l {0}
run: pytest -v tests/
22 changes: 22 additions & 0 deletions .github/workflows/precommit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: pre-commit

on:
push:
branches: "*"
pull_request:
branches: master

jobs:
pre-commit:
name: pre-commit
runs-on: ubuntu-latest

steps:
- name: checkout
uses: actions/checkout@v2
- name: set up python
uses: actions/setup-python@v2
with:
python-version: 3.8
- name: Run pre-commit
uses: pre-commit/action@v2.0.0
25 changes: 25 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml

- repo: https://github.com/ambv/black
rev: 20.8b1
hooks:
- id: black
args: []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add the following pre-commit hook for formatting notebooks as well:

  - repo: https://github.com/deathbeds/prenotebook
    rev: f5bdb72a400f1a56fe88109936c83aa12cc349fa
    hooks:
      - id: prenotebook
        args:
          [
            '--keep-output',
            '--keep-metadata',
            '--keep-execution-count',
            '--keep-empty',
          ]


- repo: https://github.com/deathbeds/prenotebook
rev: f5bdb72a400f1a56fe88109936c83aa12cc349fa
hooks:
- id: prenotebook
args:
[
'--keep-output',
'--keep-metadata',
'--keep-execution-count',
'--keep-empty',
]
59 changes: 59 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# HiRes-CESM-analysis

This repository is building a set of tools for analyzing BGC output in a high-resolution POP run.

## For Developers

A few recommended practices to incorporate in your development sandbox:

### Keep your conda environment up to date

The first time you check out this repository, run

```
$ conda env install -f environments/environment.yaml
```

If you notice the YAML file has changed after you fetch changes from github,
update the environment with

```
$ conda env update -f environments/environment.yaml
```

### Use `pre-commit` to test code before commiting

Please take advantage of the pre-commit package to ensure that `black` is run before commiting:

```
$ pre-commit install --install-hooks # set up pre-commit
$ pre-commit run -a # check all the files currently in the repo
```

The pre-commit package is already installed via the `hires-marbl` conda environment.
There is a github action to run `black` on all pull requests,
but running it locally via-pre-commit will reduce the number of failed actions.
NOTE: for some reason, to properly install `pre-commit` on the CISL systems,
the above command must be run from `casper` rather than `cheyenne`.

Note that pre-commit creates a virtual environment using specific tags of each package.
To ensure pre-commit is testing with the latest releases, it is necessary to run

```
$ pre-commit autoupdate
$ pre-commit install --install-hooks # set up pre-commit
```

from time to time.

### Run `pytest` after modifying python in `utils/`

To test some of the python code in `notebooks/utils/`, run `pytest`.
These tests can be run from the top level of this repository by running

```
$ pytest tests/
```

If you add new code to this directory,
consider writing small tests to ensure it is running as expected.
8 changes: 5 additions & 3 deletions environments/environment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ dependencies:
- dask-jobqueue
- dask-mpi
- distributed

# development
- black
- pre-commit
- pytest

- pip:
- ncar-jobqueue
- ncar-jobqueue
- black
47 changes: 30 additions & 17 deletions notebooks/Pull info from logs.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@
"outputs": [],
"source": [
"cases = dict()\n",
"cases['001'] = utils.CaseClass(['g.e22.G1850ECO_JRA_HR.TL319_t13.001', 'g.e22.G1850ECO_JRA_HR.TL319_g17.001'])\n",
"cases['002'] = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.002')\n",
"cases['003'] = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.003')\n",
"cases['004'] = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.004')"
"cases[\"001\"] = utils.CaseClass(\n",
" [\n",
" \"g.e22.G1850ECO_JRA_HR.TL319_t13.001\",\n",
" \"g.e22.G1850ECO_JRA_HR.TL319_g17.001\",\n",
" ]\n",
")\n",
"cases[\"002\"] = utils.CaseClass(\"g.e22.G1850ECO_JRA_HR.TL319_t13.002\")\n",
"cases[\"003\"] = utils.CaseClass(\"g.e22.G1850ECO_JRA_HR.TL319_t13.003\")\n",
"cases[\"004\"] = utils.CaseClass(\"g.e22.G1850ECO_JRA_HR.TL319_t13.004\")"
]
},
{
Expand Down Expand Up @@ -171,7 +176,7 @@
"source": [
"# get_co2calc_warning_cnt now counts each it value separately\n",
"# So [292, 204, 82, 1] => 292 warnings at it=1, 204 at it=2, 82 at it=3, and 1 at it=4\n",
"cases['002'].get_co2calc_warning_cnt()"
"cases[\"002\"].get_co2calc_warning_cnt()"
]
},
{
Expand All @@ -193,9 +198,11 @@
}
],
"source": [
"utils.plot_dict_with_date_keys(cases['001'].get_co2calc_warning_cnt(),\n",
" title='co2calc warnings',\n",
" legend=[f'it = {it}' for it in range(1,5)])"
"utils.plot_dict_with_date_keys(\n",
" cases[\"001\"].get_co2calc_warning_cnt(),\n",
" title=\"co2calc warnings\",\n",
" legend=[f\"it = {it}\" for it in range(1, 5)],\n",
")"
]
},
{
Expand All @@ -217,9 +224,11 @@
}
],
"source": [
"utils.plot_dict_with_date_keys(cases['002'].get_co2calc_warning_cnt(),\n",
" title='co2calc warnings',\n",
" legend=[f'it = {it}' for it in range(1,5)])"
"utils.plot_dict_with_date_keys(\n",
" cases[\"002\"].get_co2calc_warning_cnt(),\n",
" title=\"co2calc warnings\",\n",
" legend=[f\"it = {it}\" for it in range(1, 5)],\n",
")"
]
},
{
Expand All @@ -241,9 +250,11 @@
}
],
"source": [
"utils.plot_dict_with_date_keys(cases['003'].get_co2calc_warning_cnt(),\n",
" title='co2calc warnings',\n",
" legend=[f'it = {it}' for it in range(1,5)])"
"utils.plot_dict_with_date_keys(\n",
" cases[\"003\"].get_co2calc_warning_cnt(),\n",
" title=\"co2calc warnings\",\n",
" legend=[f\"it = {it}\" for it in range(1, 5)],\n",
")"
]
},
{
Expand All @@ -265,9 +276,11 @@
}
],
"source": [
"utils.plot_dict_with_date_keys(cases['004'].get_co2calc_warning_cnt(),\n",
" title='co2calc warnings',\n",
" legend=[f'it = {it}' for it in range(1,5)])"
"utils.plot_dict_with_date_keys(\n",
" cases[\"004\"].get_co2calc_warning_cnt(),\n",
" title=\"co2calc warnings\",\n",
" legend=[f\"it = {it}\" for it in range(1, 5)],\n",
")"
]
}
],
Expand Down
42 changes: 32 additions & 10 deletions notebooks/Sanity Check.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,26 @@
"source": [
"cases = dict()\n",
"\n",
"cases['001'] = utils.CaseClass(['g.e22.G1850ECO_JRA_HR.TL319_t13.001', 'g.e22.G1850ECO_JRA_HR.TL319_g17.001'])\n",
"cases['001']._open_history_files('pop.h.nday1')\n",
"cases[\"001\"] = utils.CaseClass(\n",
" [\n",
" \"g.e22.G1850ECO_JRA_HR.TL319_t13.001\",\n",
" \"g.e22.G1850ECO_JRA_HR.TL319_g17.001\",\n",
" ]\n",
")\n",
"cases[\"001\"]._open_history_files(\"pop.h.nday1\")\n",
"\n",
"cases['002'] = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.002')\n",
"cases['002']._open_history_files('pop.h.nday1')\n",
"cases[\"002\"] = utils.CaseClass(\"g.e22.G1850ECO_JRA_HR.TL319_t13.002\")\n",
"cases[\"002\"]._open_history_files(\"pop.h.nday1\")\n",
"\n",
"cases['003'] = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.003', end_date = '0001-05')\n",
"cases['003']._open_history_files('pop.h.nday1')\n",
"cases[\"003\"] = utils.CaseClass(\n",
" \"g.e22.G1850ECO_JRA_HR.TL319_t13.003\", end_date=\"0001-05\"\n",
")\n",
"cases[\"003\"]._open_history_files(\"pop.h.nday1\")\n",
"\n",
"cases['004'] = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.004', end_date = '0001-05')\n",
"cases['004']._open_history_files('pop.h.nday1')"
"cases[\"004\"] = utils.CaseClass(\n",
" \"g.e22.G1850ECO_JRA_HR.TL319_t13.004\", end_date=\"0001-05\"\n",
")\n",
"cases[\"004\"]._open_history_files(\"pop.h.nday1\")"
]
},
{
Expand All @@ -68,7 +77,14 @@
}
],
"source": [
"fig = utils.compare_fields_at_lat_lon([cases['001'], cases['002'], cases['003'], cases['004']], 'HMXL_2', 'pop.h.nday1', nlat=142, nlon=897, individual_plots=True);"
"fig = utils.compare_fields_at_lat_lon(\n",
" [cases[\"001\"], cases[\"002\"], cases[\"003\"], cases[\"004\"]],\n",
" \"HMXL_2\",\n",
" \"pop.h.nday1\",\n",
" nlat=142,\n",
" nlon=897,\n",
" individual_plots=True,\n",
")"
]
},
{
Expand All @@ -90,7 +106,13 @@
}
],
"source": [
"fig = utils.compare_fields_at_lat_lon([cases['001'], cases['002'], cases['003'], cases['004']], 'HMXL_2', 'pop.h.nday1', nlat=142, nlon=897);"
"fig = utils.compare_fields_at_lat_lon(\n",
" [cases[\"001\"], cases[\"002\"], cases[\"003\"], cases[\"004\"]],\n",
" \"HMXL_2\",\n",
" \"pop.h.nday1\",\n",
" nlat=142,\n",
" nlon=897,\n",
")"
]
},
{
Expand Down
4 changes: 2 additions & 2 deletions notebooks/plot_suite_003.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"\n",
" utils.summary_plot_histogram(da, var_metadata)\n",
"\n",
" utils.summary_plot_maps(da, var_metadata)\n"
" utils.summary_plot_maps(da, var_metadata)"
]
},
{
Expand Down Expand Up @@ -2913,7 +2913,7 @@
}
],
"source": [
"case = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.003')\n",
"case = utils.CaseClass(\"g.e22.G1850ECO_JRA_HR.TL319_t13.003\")\n",
"stream = \"pop.h\"\n",
"case._open_history_files(stream)\n",
"for varname in var_metadata_all:\n",
Expand Down
4 changes: 2 additions & 2 deletions notebooks/plot_suite_004.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"\n",
" utils.summary_plot_histogram(da, var_metadata)\n",
"\n",
" utils.summary_plot_maps(da, var_metadata)\n"
" utils.summary_plot_maps(da, var_metadata)"
]
},
{
Expand Down Expand Up @@ -3117,7 +3117,7 @@
}
],
"source": [
"case = utils.CaseClass('g.e22.G1850ECO_JRA_HR.TL319_t13.004')\n",
"case = utils.CaseClass(\"g.e22.G1850ECO_JRA_HR.TL319_t13.004\")\n",
"stream = \"pop.h\"\n",
"case._open_history_files(stream)\n",
"for varname in var_metadata_all:\n",
Expand Down
Loading