-
-
Notifications
You must be signed in to change notification settings - Fork 568
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
Remove brew-sundials from the recommended developer installations #2925
Remove brew-sundials from the recommended developer installations #2925
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## develop #2925 +/- ##
========================================
Coverage 99.68% 99.68%
========================================
Files 273 273
Lines 18997 18997
========================================
Hits 18938 18938
Misses 59 59 ☔ View full report in Codecov by Sentry. |
For Linux installation, and in addition to This can remove the need for the note about the error |
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, but I don't use Mac, so I have requested a review from Valentin who uses 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.
Looks good except changelog needs updating
CHANGELOG.md
Outdated
@@ -21,6 +21,7 @@ | |||
|
|||
## Bug fixes | |||
|
|||
- Remove brew install for Mac from the recommended developer installation options for SUNDIALS ([#2925](https://github.com/pybamm-team/PyBaMM/pull/2925)) |
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 needs to be moved to the new section
Description
The (Mac recommended)
brew
install procedure for sundials does not link the cmake args that are required bytox -e dev
. Here, we simplify the developer installation procedure for Mac, bringing it in-line with that for linux, by usingtox -e pybamm-requires
to download, compile and install the SuiteSparse and SUNDIALS libraries.Fixes #2897
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.
Key checklist:
(documentation change only)
[ ] No style issues:$ pre-commit run
(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
$ python run-tests.py --doctest
Further checks: