-
Notifications
You must be signed in to change notification settings - Fork 0
Maintenance updates #94
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
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
f736547 to
4a37e41
Compare
corviday
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I especially like all the poe scripts and the documentation of them, a lot of thought went into that.
| # Development | ||
|
|
||
| This `docker-compose` is set up for development purposes. It will up a dev instance of `quail` with production settings. All you are required to do is to add the following lines to `birdhouse-config/env.local` running on `dev03`: | ||
| This `docker-compose` is set up for development purposes. It will up a dev instance of `quail` with production settings. All you are required to do is to add the following lines to `birdhouse-config/env.local` running on `marble-dev01`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| - Push it: `git push` | ||
| - Push tag: `git push --tags` | ||
|
|
||
| See the [bumpversion](https://pypi.org/project/bumpversion/) documentation for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, cool, I didn't know about that.
| prepare-notebooks Download the output sanitizer config for notebook tests | ||
| test-notebooks Run all local notebook tests with sanitized output | ||
| test-notebooks-local Run notebook tests locally using nbval with sanitization | ||
| docs Convert notebooks to HTML for documentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of tasks, good documentation of them. 👍
| name = "quail" | ||
| version = "0.7.1" | ||
| description = "A Web Processing Service for Climate Data Analysis." | ||
| authors = [{ name = "Nikola Rados", email = "nrados@uvic.ca" }] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps add yourself as an author?
| env = { LOCAL_URL = "http://localhost:5000" } | ||
| help = "Run notebook tests locally using nbval with sanitization" | ||
|
|
||
| # [tool.poe.tasks.test-notebooks-prod] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left them commented out because local manual runs are more practical for development. Redeploying to Marble is tedious and nbval sometimes shows failures that pass fine in JupyterLab.
| "output_type": "stream", | ||
| "text": [ | ||
| "Using quail on https://docker-dev03.pcic.uvic.ca/twitcher/ows/proxy/quail/wps\n" | ||
| "Using quail on https://marble-dev01.pcic.uvic.ca/twitcher/ows/proxy/quail/wps\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a question for another PR, but should this be configured dynamically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This essentially is, when viewed through the notebook preview it is the output text from running:
url = get_target_url("quail")
print(f"Using quail on {url}")
| { | ||
| "cell_type": "code", | ||
| "execution_count": 1, | ||
| "execution_count": 12, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just curious what the deal with all the changed execution_counts is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is metadata referring to how many times a cell has executed. This could be relevant for non-linear executing notebooks, but it isn't important here. We could create a pre-commit action to clean up metadata and simplify future reviews. I'll open an issue for that improvement.
Pending: