PyBaMM
PyBaMM copied to clipboard
Removed Google colab error warning.
Description
Follow-up of #3740
Fixes # (issue)
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Optimization (back-end change that speeds up the code)
- [x] Bug fix (non-breaking change which fixes an issue)
Key checklist:
- [x] No style issues:
$ pre-commit run
(or$ nox -s pre-commit
) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code) - [ ] All tests pass:
$ python run-tests.py --all
(or$ nox -s tests
) - [x] The documentation builds:
$ python run-tests.py --doctest
(or$ nox -s doctests
)
You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick
(or $ nox -s quick
).
Further checks:
- [ ] Code is commented, particularly in hard-to-understand areas
- [ ] Tests added that prove fix is effective or that feature works
Thanks @prady0t, could you remove these lines:
# Temporary fix for Python 3.12 CI. TODO: remove after
# https://bitbucket.org/pybtex-devs/pybtex/issues/169/replace-pkg_resources-with
# is fixed
session.install("setuptools", silent=False)
wherever they exist in noxfile.py
?
Sorry for making changes lazily. 🙂 I've updated the PR.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 99.60%. Comparing base (
8056d22
) to head (abc7cc6
).
Additional details and impacted files
@@ Coverage Diff @@
## develop #3886 +/- ##
===========================================
- Coverage 99.60% 99.60% -0.01%
===========================================
Files 259 259
Lines 21273 21268 -5
===========================================
- Hits 21189 21184 -5
Misses 84 84
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Also, it looks like the doctests on Python 3.12 are failing (strange) but all other jobs on Python 3.12 pass. Could we re-add the setuptools
installation to the doctests session for now?
Also, it looks like the doctests on Python 3.12 are failing (strange) but all other jobs on Python 3.12 pass. Could we re-add the
setuptools
installation to the doctests session for now?
It could be a latex parsing issue: SyntaxWarning: invalid escape sequence '\m'
is caused by self.spatial_unit = "$\mu$m"
Maybe we need to make them raw strings or something:
self.spatial_unit = r"$\mu$m"
Yes, that warning would be good to fix too. But the failure seems to be coming from a Sphinx-related warning: sphinxcontrib_bibtex
can't find pkg_resources
– even though it is enabled with setuptools
as a seed package in other sessions 😕
I've re added setuptools installation to the doctests session and removed checks for error messages.
The doctests and example scripts are still failing on Python 3.12, and I'm not sure why. We have two options:
- Add the
session.install("setuptools")
hack back again for the time being- Modify these sessions in the workflows to run on Python 3.11 instead.
I would go forward with the first option at this time, because using the latest available Python version will help us catch any related (or unrelated) issues early.
Not all of our functionality works on 3.12, so 3.11 is probably better
Not all of our functionality works on 3.12, so 3.11 is probably better
Yes, I had considered that when posting my comment – I had checked the example scripts and none of them are currently using unsupported functionality (which is scikits.odes
for Python 3.12). So Python 3.12 would be fine (for the docs-related tests, it also helps catch warnings/errors early because most documentation dependencies are pure-Python and do not have an upper bound on their Python version requirement). So we can go ahead with either of those, but I tend to lean slightly on 3.12. Both Python versions have a long time till their EOL.
I tend to lean slightly on 3.12. Both Python versions have a long time till their EOL.
I would say that it is better to go with a version where everything runs, we can always upgrade the version later. It is for the documentation generation, not something that must be the newest python
I would say that it is better to go with a version where everything runs, we can always upgrade the version later. It is for the documentation generation, not something that must be the newest python
Sure, let's do it for just the doctests, but keep Python 3.12 for the example scripts (because it matches with the example notebooks tests).
would doing
@nox.session(name="doctests", python="3.11")
work?
would doing
@nox.session(name="doctests", python="3.11")
work?
You don't need to change the nox
configuration for the doctests. We just need to fix the CI and we should be good to go – just alter test_on_push.yml
to use Python 3.11 in the doctests job instead of Python 3.12. The only change required in noxfile.py
is to bring back the session.install("setuptools")
line that you had removed from the example scripts session.
Yess. I'll be more than happy to rework here.