-
Notifications
You must be signed in to change notification settings - Fork 113
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
Module 5 updates #109
Module 5 updates #109
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
One query RE behaviour (in notebook).
One link in 08_02 is broken but may be part of PR
"rootdir: /home/turingdev/research-software/rse-course/ch03tests/diffusion\n", | ||
"plugins: cov-2.12.1, anyio-3.3.0\n", | ||
"collected 1 item\n", | ||
"platform darwin -- Python 3.9.9, pytest-7.1.2, pluggy-1.0.0\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.
Is the intended behaviour here for the test to "pass" as previously or to give an error message?
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 think it's failing because of the blank
def test_energy():
# Test something
that's created in the cell above. Adding a single "return" or "..." line should fix it - would you mind adding it?
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.
What's the broken link?
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.
pass
is a good no-op in Python if you need one.
@@ -920,7 +920,7 @@ | |||
"cell_type": "markdown", | |||
"metadata": {}, | |||
"source": [ | |||
"[We have seen before](../ch01data/060files.html#Closing-files) [[notebook](../ch01data/060files.ipynb#Closing-files)] that, instead of separately `open`ing and `close`ing a file, we can have\n", | |||
"[We have seen before](../module02_intermediate_python/02_04_working_with_files.html#Closing-files) [[notebook](../module02_intermediate_python/02_04_working_with_files.ipynb#Closing-files)] that, instead of separately `open`ing and `close`ing a file, we can have\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.
@jack89roberts hopefully this fixes the broken links?
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.
should do 👍
nose
withpytest
everywhere#88