-
-
Notifications
You must be signed in to change notification settings - Fork 558
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
Final pytest migration #4443
Final pytest migration #4443
Conversation
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
…yBaMM into pytest-migration-final Merging
Just two files left |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4443 +/- ##
========================================
Coverage 99.46% 99.46%
========================================
Files 293 293
Lines 22332 22332
========================================
Hits 22212 22212
Misses 120 120 ☔ View full report in Codecov by Sentry. |
Unfortunately, there's no standard way to replace import io
import logging
log_capture = io.StringIO()
handler = logging.StreamHandler(log_capture)
handler.setLevel(logging.ERROR)
logger = logging.getLogger('pybamm.logger')
logger.addHandler(handler) and then, if you run the code that should produce the logger warning: |
@prady0t, how is this going? The tests are all already passing here, and the approach I suggested should be okay in general – it works on my machine. Please let me know if you need additional help with this. |
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Hi. I've updated the files. Lets wait for CI to run. |
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
…yBaMM into pytest-migration-final Merging
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.
Thanks, @prady0t – I guess the next step to do after this PR should be to disallow the use of unittest
through the Ruff rule as we discussed earlier
Edit: I notice some failing tests with the parameterisation, could you try fixing them locally?
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
@agriyakhetarpal I've updated this PR. |
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
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.
Thanks for completing the migration, @prady0t!
WIP