-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
Fix Lychee link failures from stack overflow #3989
Changes from all commits
af6771b
31dd7ed
18be562
22a73b8
3732de5
c5a5802
79dced6
976569b
a5ed681
8e6ac2e
a4c127b
e59c12a
8ac9977
0b93e75
4d70768
4607ca1
3d6d71b
5caace3
dd1f645
f663291
99376b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,3 @@ | ||
# | ||
# Base class for callbacks and some useful callbacks for pybamm | ||
# Callbacks are used to perform actions (e.g. logging, saving) | ||
# at certain points in the simulation | ||
# Inspired by Keras callbacks | ||
# https://github.com/keras-team/keras/blob/master/keras/callbacks/callback.py | ||
# | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we replace this with the updated link? It is often useful to look at the original implementations, especially for people trying to extend PyBaMM (or any software) and not just use it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replacing with a GitHub permalink (pinned to a commit hash) would also be helpful, so that we can know what the implementation was at the time of writing and the link won't break. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to mark were we were inspired by other projects. Especially when concepts like callbacks are pretty common. We should use links only where it is necessary. The fact is that this keras link broke just 6 months ago too and this time it was not appearing in the link checker log until the stackoverflow stuff was removed. |
||
import pybamm | ||
import numpy as np | ||
import inspect | ||
|
@@ -99,8 +92,7 @@ class CallbackList(Callback): | |
|
||
This is done without having to redefine the method each time by using the | ||
`callback_loop_decorator` decorator, which is applied to every method that starts | ||
with `on_`, using the `inspect` module. See | ||
https://stackoverflow.com/questions/1367514/how-to-decorate-a-method-inside-a-class. | ||
with `on_`, using the `inspect` module. | ||
|
||
If better control over how the callbacks are called is required, it might be better | ||
to be more explicit with the for loop. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,8 +14,9 @@ | |
|
||
class NumpyEncoder(json.JSONEncoder): | ||
""" | ||
Numpy serialiser helper class that converts numpy arrays to a list | ||
https://stackoverflow.com/questions/26646362/numpy-array-is-not-json-serializable | ||
Comment on lines
-17
to
-18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added plain text that explains, so we don't need the link anymore. If the explanation is not good enough then we can just update the text |
||
Numpy serialiser helper class that converts numpy arrays to a list. | ||
Numpy arrays cannot be directly converted to JSON, so the arrays are | ||
converted to python list objects before encoding. | ||
""" | ||
|
||
def default(self, obj): | ||
|
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'll still prefer attributing StackOverflow answers. Moreover, in this case, the question was asked by @agriyakhetarpal on StackOverflow to specifically cater to the needs of PyBaMM.