with when opening a file
#24993
When manipulating a file in Python without using with(), you have to close the file manually, so this PR does it in get_block_hashes (contrib/linearize/linearize-data.py).
Edit: this PR does it for all occurances that previously weren’t using with.
When manipulating a file in Python without using with(),
You’re right, though prefer fixing this by using with …. Or is there a specific reason to avoid it here?
You’re right, though prefer fixing this by using with …. Or is there a specific reason to avoid it here? @laanwj, I didn’t do it before because since in other places it closes the file after manipulating it (without using
with), it would be simple just add aclose()here, but I 100% preferwith, can I modify all the occurances to usewith?
You’re right, though prefer fixing this by using with …. Or is there a specific reason to avoid it here?
@laanwj, I didn’t do it before because since in other places it closes the file after manipulating it (without using
with), it would be simple just add aclose()here, but I 100% preferwith, can I modify all the occurances to usewith?
Concept ACK on identifying further such cases and fixing them. Especially for small scripts not closing is not a big deal in practice though; here is a nice list of arguments why it it strongly recommended to do it anyway: https://stackoverflow.com/a/25070939
Assuming that the following used regex is reliable, we have 17 occurences of open() calls without the with syntax (didn’t check them in detail, but I guess many of them never call close):
0$ git grep "=[ ]*open(" ./contrib/ ./test | wc -l
1 17
2$ git grep "=[ ]*open(" ./contrib/ ./test
3contrib/devtools/copyright_header.py: f = open(filename, 'r', encoding="utf8")
4contrib/devtools/copyright_header.py: f = open(filename, 'w', encoding="utf8")
5contrib/linearize/linearize-data.py: f = open(settings['hashlist'], "r", encoding="utf8")
6contrib/linearize/linearize-data.py: self.outF = open(self.outFname, "wb")
7contrib/linearize/linearize-data.py: self.inF = open(fname, "rb")
8contrib/linearize/linearize-data.py: f = open(sys.argv[1], encoding="utf8")
9contrib/linearize/linearize-hashes.py: f = open(sys.argv[1], encoding="utf8")
10contrib/verify-commits/verify-commits.py: verified_root = open(dirname + "/trusted-git-root", "r", encoding="utf8").read().splitlines()[0]
11contrib/verify-commits/verify-commits.py: verified_sha512_root = open(dirname + "/trusted-sha512-root-commit", "r", encoding="utf8").read().splitlines()[0]
12contrib/verify-commits/verify-commits.py: revsig_allowed = open(dirname + "/allow-revsig-commits", "r", encoding="utf-8").read().splitlines()
13contrib/verify-commits/verify-commits.py: unclean_merge_allowed = open(dirname + "/allow-unclean-merge-commits", "r", encoding="utf-8").read().splitlines()
14contrib/verify-commits/verify-commits.py: incorrect_sha512_allowed = open(dirname + "/allow-incorrect-sha512-commits", "r", encoding="utf-8").read().splitlines()
15test/functional/feature_config_args.py: conf_file_contents = open(conf_file, encoding='utf8').read()
16test/functional/feature_versionbits_warning.py: alert_text = open(self.alert_filename, 'r', encoding='utf8').read()
17test/util/test_runner.py: raw_data = open(input_filename, encoding="utf8").read()
18test/util/test_runner.py: inputData = open(filename, encoding="utf8").read()
19test/util/test_runner.py: outputData = open(os.path.join(testDir, outputFn), encoding="utf8").read()
can I modify all the occurances to use with?
Sounds good to me. It removes the burden of having the explicitly think about closing (removes the entire bug class of “forget to close”) so is very much preferred when it’s possible.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
when when opening files in Python. What do you think?
Code review ACK 027aab663aaca32818051d456c3900326377281c
I think we could put in the docs a recommendation to use when when opening files in Python. What do you think?
Not sure. Isn’t that like a general Python thing, not specific to our project? I don’t think it’s worthwhile to maintain a document of basic Python programming language best practices in our project.
