OSGeo
man: Use a context manager for opening files
In the same line of thought of other PRs of the moment fixing ResourceWarnings of unclosed files, this also applies the same context manager pattern when used on multiple lines + removing the f.close(), or using pathlib's Path().read_text()/ Path().write_text() when the whole file is acted upon in one step.
I am seeing these when trying to make the same script that OSGeo4W uses to package grass, (since it runs the manual creation).
Let me suggest @landam to review, also to avoid conflicts with the upcoming markdown changes in #3849
Let me suggest @landam to review, also to avoid conflicts with the upcoming markdown changes in #3849
I see that some of the files have context managers in his PR, but in the meantime until that https://github.com/OSGeo/grass/pull/3849 PR is ready, we still have this to handle. It will have a conflict to merge, but the diff will be smaller after.
Given the anticipated conflicts with the Markdown doc work and the fact that the build now fails, what about just merging the per-file exclusions for now?
Y'a I saw these failures.. automatic merging must have messed up somewhere, as only the pyproject.toml had conflicts that I manually solved (they were hard, since the INT001,INT002,INT003 exclusions were gone).
What about creating a PR for the exclusions, and one for the parts that the markdown docs PR doesn't touch (if they are some). Because in the meantime, we see hundreds (maybe even thousands) of warnings on builds, visible on windows for example (because I was working with them). It's not any better to delay some work on the hope that an incomplete PR might be merged someday.
I guess when the time comes for #3849 to be merged, it can replace the conflicting files with its version and go from there rather than attempting to merge the file versions together. Is that what you are suggesting @echoix?
I guess when the time comes for #3849 to be merged, it can replace the conflicting files with its version and go from there rather than attempting to merge the file versions together. Is that what you are suggesting @echoix?
That's a way to think about it, I didn't think exactly this way, but it can make sense
When I locally do distclean, the subsequent build fails in man. Building again, in man or in the root dir makes the build succeed, but it is always the second build (I reproduced more than once, but didn't get to the error yet).
Do you happen to know what file was complaining? I'm quite surprised, as it's mostly a simple pattern applied, except for the places where we use pathlib instead, which we successfully use elsewhere. Does any change in man followed by a distclean make the second build fail (thus suggesting a problem in makefiles), or it is only with these changes specifically?
If we are unsure, I can try again from scratch, and split it in multiple PRs, one per pattern used, to know where you get this.
It's weird to me
I get:
GISBASE="/home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu" ARCH="x86_64-pc-linux-gnu" ARCH_DISTDIR="/home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu" VERSION_NUMBER=8.5.0dev VERSION_DATE=2024 python3 ./build_class_graphical.py /home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu/docs/html
touch /home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu/docs/html/class_graphical.html
GISBASE="/home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu" ARCH="x86_64-pc-linux-gnu" ARCH_DISTDIR="/home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu" VERSION_NUMBER=8.5.0dev VERSION_DATE=2024 python3 ./parser_standard_options.py -t "/home/vpetras/Projects/grass/code/grass/lib/gis/parser_standard_options.c" -f "grass" -o "/home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu/docs/html/parser_standard_options.html" -p 'id="opts_table" class="scroolTable"'
Traceback (most recent call last):
File "./parser_standard_options.py", line 222, in <module>
options = OptTable(parse_options(cfile.readlines(), startswith=args.startswith))
File "./parser_standard_options.py", line 107, in parse_options
res = parse_glines(glines)
File "./parser_standard_options.py", line 68, in parse_glines
key, default = (w.strip() for w in split_opt_line(line[5:]))
File "./parser_standard_options.py", line 51, in split_opt_line
default = default.removeprefix("_(")
AttributeError: 'str' object has no attribute 'removeprefix'
make[3]: *** [Makefile:131: /home/vpetras/Projects/grass/code/grass/dist.x86_64-pc-linux-gnu/docs/html/parser_standard_options.html] Error 1
make[3]: Leaving directory '/home/vpetras/Projects/grass/code/grass/man'
make[2]: *** [Makefile:48: default] Error 2
make[2]: Leaving directory '/home/vpetras/Projects/grass/code/grass/man'
I didn't look more into that, but that's the only "error:" in the output (not counting ctypes).
Ah, that should be a simple one: str always has removeprefix since it was introduced in 3.9, our minimum supported Python version for the main (dev) branch for 8.5. So you are using a not supported Python version, it's all.
It's not even from that PR, but from this weekend (of a PR filed 5 days ago): https://github.com/OSGeo/grass/commit/560340a9c01414469996b3f94d60406d1297e2c5
It wasn't problematic as "we don't support Python 3.8" since mid June on the main branch, but was changed in the pyproject.toml only later on, that allows tools, and even black to use syntax that is only allowed in later versions. (For example the parenthesized grouped with statements in 3.9 grammar and supported, but announced in 3.10 with the new Python parser)
It wasn't problematic as "we don't support Python 3.8" since mid June on the main branch, but was changed in the pyproject.toml only later on, that allows tools, and even black to use syntax that is only allowed in later versions. (For example the parenthesized grouped with statements in 3.9 grammar and supported, but announced in 3.10 with the new Python parser)
I realize that I never applied that upgrade to black specifically, only ruff and other tools that respect the project-level Python version.
I drafted something out, but just need to apply one change that black prevented itself from doing since it needed to support Python 3.8: https://github.com/echoix/grass/pull/235
https://github.com/OSGeo/grass/pull/3849 was merged, and this PR was reworked against the changes, and its ready
@neteler Would you mind taking a look at this whilst we've taken the week to work on docs recently? This should be solving most if not all of the resourcewarnings that we see when the docs are compiled. It's been a while and I continued to keep it updated.