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

Fix Lychee link failures from stack overflow #3989

Merged
merged 21 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ body:
id: reproduce
attributes:
label: Steps to Reproduce
description: Tell us how to reproduce this behaviour. Ideally, this should take the form of a [Minimum Workable Example](https://stackoverflow.com/help/minimal-reproducible-example)
description: Tell us how to reproduce this behaviour. Ideally, this should include a code block which produces the error. Strive to make this example as small and simple as possible. It should contain the code required to reproduce the error, and no additional code. For example, if your code includes lines to run a simulation, then lines to plot the results, and the lines to run the simulation fail, only include those lines (and not the plotting lines, which are irrelevant). Often, the act of simplifying code to pinpoint errors can help you find bugs in your own code. For more information, see [these references](https://en.wikipedia.org/wiki/Minimal_reproducible_example#External_links)
validations:
required: true
- type: textarea
Expand Down
12 changes: 1 addition & 11 deletions .readthedocs.yaml
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
# .readthedocs.yaml
# Read the Docs configuration file
# See https://docs.readthedocs.io/en/stable/config-file/v2.html for details

# Required
version: 2

# Build documentation in the docs/ directory with Sphinx
Expand All @@ -25,16 +20,11 @@ build:
os: ubuntu-22.04
tools:
python: "3.12"
# You can also specify other tool versions:
# nodejs: "19"
# rust: "1.64"
# golang: "1.19"
jobs:
# Unshallow the git clone otherwise this may cause issues with Sphinx extensions
post_checkout:
- git fetch --unshallow
# Altered PDF build and upload job, attributed to
# https://stackoverflow.com/a/76992101/14001839
# Altered PDF build and upload job
Copy link
Member

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.

# This also runs on PR builds, but does not upload the PDF
post_build:
- mkdir --parents $READTHEDOCS_OUTPUT/pdf/
Expand Down
14 changes: 11 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,12 @@ You now have everything you need to start making changes!

### B. Writing your code

6. PyBaMM is developed in [Python](https://en.wikipedia.org/wiki/Python_(programming_language)), and makes heavy use of [NumPy](https://en.wikipedia.org/wiki/NumPy) (see also [NumPy for MatLab users](https://numpy.org/doc/stable/user/numpy-for-matlab-users.html) and [Python for R users](https://www.rebeccabarter.com/blog/2023-09-11-from_r_to_python)).
6. PyBaMM is developed in [Python](https://www.python.org)), and makes heavy use of [NumPy](https://numpy.org/) (see also [NumPy for MatLab users](https://numpy.org/doc/stable/user/numpy-for-matlab-users.html) and [Python for R users](https://www.rebeccabarter.com/blog/2023-09-11-from_r_to_python)).
7. Make sure to follow our [coding style guidelines](#coding-style-guidelines).
8. Commit your changes to your branch with [useful, descriptive commit messages](https://chris.beams.io/posts/git-commit/): Remember these are publicly visible and should still make sense a few months ahead in time. While developing, you can keep using the GitHub issue you're working on as a place for discussion. [Refer to your commits](https://stackoverflow.com/questions/8910271/how-can-i-reference-a-commit-in-an-issue-comment-on-github) when discussing specific lines of code.
8. Commit your changes to your branch with [useful, descriptive commit messages](https://chris.beams.io/posts/git-commit/): Remember these are
publicly visible and should still make sense a few months ahead in time.
While developing, you can keep using the GitHub issue you're working on
as a place for discussion.
9. If you want to add a dependency on another library, or re-use code you found somewhere else, have a look at [these guidelines](#dependencies-and-reusing-code).

### C. Merging your changes with PyBaMM
Expand Down Expand Up @@ -87,7 +90,12 @@ Class names are CamelCase, and start with an upper case letter, for example `MyO
While it's a bad idea for developers to "reinvent the wheel", it's important for users to get a _reasonably sized download and an easy install_. In addition, external libraries can sometimes cease to be supported, and when they contain bugs it might take a while before fixes become available as automatic downloads to PyBaMM users.
For these reasons, all dependencies in PyBaMM should be thought about carefully, and discussed on GitHub.

Direct inclusion of code from other packages is possible, as long as their license permits it and is compatible with ours, but again should be considered carefully and discussed in the group. Snippets from blogs and [stackoverflow](https://stackoverflow.com/) can often be included without attribution, but if they solve a particularly nasty problem (or are very hard to read) it's often a good idea to attribute (and document) them, by making a comment with a link in the source code.
Direct inclusion of code from other packages is possible, as long as their
license permits it and is compatible with ours, but again should be
considered carefully and discussed in the group. Snippets from blogs and
stackoverflow can often be included without attribution, but if they solve a
particularly nasty problem (or are very hard to read) it's often a good idea to
attribute (and document) them, by making a comment with a link in the source code.

### Separating dependencies

Expand Down
10 changes: 1 addition & 9 deletions pybamm/callbacks.py
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
#
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion pybamm/citations.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ def _caller_name():
Returns the qualified name of classes that call :meth:`register` internally.
Gets cached in order to reduce the number of calls.
"""
# Attributed to https://stackoverflow.com/a/53490973
caller_name = _getframe().f_back.f_back.f_locals["self"].__class__.__qualname__
return caller_name

Expand Down
8 changes: 0 additions & 8 deletions pybamm/logger.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
#
# Logging class for PyBaMM
# Includes additional logging levels inspired by verboselogs
# https://pypi.org/project/verboselogs/#overview-of-logging-levels
#
# Implementation from stackoverflow
# https://stackoverflow.com/questions/2183233/how-to-add-a-custom-loglevel-to-pythons-logging-facility
#
import logging


Expand Down
2 changes: 1 addition & 1 deletion pybamm/solvers/base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ def solve(
)
# It is assumed that when len(inputs_list) > 1, model set
# up (initial condition, time-scale and length-scale) does
# not depend on input parameters. Thefore only `model_inputs[0]`
# not depend on input parameters. Therefore, only `model_inputs[0]`
# is passed to `set_up`.
# See https://github.com/pybamm-team/PyBaMM/pull/1261
self.set_up(model, model_inputs_list[0], t_eval)
Expand Down
5 changes: 3 additions & 2 deletions pybamm/solvers/solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Ditto!

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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):
Expand Down