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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: replacing tmpdir
with tmp_path
in tests_pytorch/checkpointing
#19642
Conversation
for more information, see https://pre-commit.ci
@fnhirwa Thanks for the PRs. Just to clarify, and to set the expectations: Find + replace won't be enough in most cases. We are not just renaming the variable, but this switches to a different path object (pathlib.Path). Fixes in the code will have to be made in many cases. See my commit that I pushed for an example how to update the code. The easiest way to know where you need updates is to run the tests locally and see which ones fail:
|
I tried to run some tests locally and they were not successfully running I think it's because I didn't install the requirements 馃檮 |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #19642 +/- ##
==========================================
- Coverage 84% 53% -31%
==========================================
Files 424 416 -8
Lines 34907 34754 -153
==========================================
- Hits 29351 18436 -10915
- Misses 5556 16318 +10762 |
It is not necessary to make everything pass before you open the PR. I just wanted to raise awareness in case you didn't know. We can of course help if you get stuck. |
What does this PR do?
Part of #16188 (comment)
changed
tmpdir
totmp_path
fixture intests_pytorch/checkpointing/
Fixes #<issue_number>
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Reviewer checklist
馃摎 Documentation preview 馃摎: https://pytorch-lightning--19642.org.readthedocs.build/en/19642/