-
-
Notifications
You must be signed in to change notification settings - Fork 553
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
Dockerize PyBaMM #3223
Dockerize PyBaMM #3223
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #3223 +/- ##
===========================================
- Coverage 99.71% 99.70% -0.01%
===========================================
Files 248 248
Lines 18871 18890 +19
===========================================
+ Hits 18817 18835 +18
- Misses 54 55 +1 ☔ View full report in Codecov by Sentry. |
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 awesome, thanks, @arjxn-py!! We would need some docs for this, maybe a new "Install using Docker" page with the contents discussed in the meeting.
Co-authored-by: Saransh Chopra <[email protected]>
…into DockerizePybamm
gabrieldemarmiesse/python-on-whales looks like a nice project that we can end up using for a Python CLI script wrapper over this Dockerfile, titled |
de4c28d
to
9991cbc
Compare
6b384e6
to
83c7df0
Compare
Co-authored-by: Saransh Chopra <[email protected]>
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.
A few comments
Co-authored-by: Saransh Chopra <[email protected]>
…into DockerizePybamm
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, thanks!
pre-commit.ci run |
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.
Amazing work, thanks @arjxn-py 🥳
One final comment -
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.
Great work, @arjxn-py! :)
Some documentation suggestions -
Co-authored-by: Saransh Chopra <[email protected]>
ad6038e
to
a058467
Compare
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #1926
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:
$ pre-commit run
(see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all
$ python run-tests.py --doctest
You can run unit and doctests together at once, using
$ python run-tests.py --quick
.Further checks: