Use pathlib

[The-Compiler]

We could use pathlib to make some stuff cleaner/easier:

https://docs.python.org/3/library/pathlib.html

[The-Compiler]

pytest now also has a tmp_path fixture which uses pathlib instead of their own py.path: https://docs.pytest.org/en/latest/tmpdir.html

[The-Compiler]

This should be relatively straightforward to introduce, at least in some places. If someone picks this up, please split it up into smaller changes/PRs (e.g. one per file or maybe subdirectory) rather than one big one.

[rbong]

I'd like to try my hand at this if no one has started yet.

[ju-sh]

I am interested in working here too. Would you guys tell me which ones you've already done so we won't be doing the same thing?

[The-Compiler]

I haven't heard back from @rbong so far, so I guess this is okay to take. Nothing has been done so far really. I'd recommend you start inside scripts/ or tests/ (changing from tmpdir to tmp_path wherever possible).

[rbong]

This is okay to take. I was still figuring out how I was going to split this up.

[The-Compiler]

Agreed, that's a good question. I'd like to avoid having one big refactoring PR, especially because that's likely to introduce even more conflicts with the already big PR backlog. Thus my suggestion to start with scripts/ and tests/ first rather than the main qutebrowser/ source tree.

Inside qutebrowser/, the majority of the work probably is getting standarddir.config() and friends to return a pathlib object. But as soon as you do that, there's probably quite a long tail of changes to be made all across the codebase.

[ju-sh]

Cool.I've started with scripts/asciidoc2html.py

And can you tell how we can test to ensure that the changes that we make won't break anything?

I figure the answer is somewhere in https://github.com/qutebrowser/qutebrowser/blob/master/doc/contributing.asciidoc#running-specific-tests but couldn't find tests for the scripts.

[The-Compiler]

There are only very few tests for scripts. For asciidoc2html.py, I'd recommend just running it and checking the HTML files it generates (and also check --help for different ways it can be run).

[ju-sh]

Hi. Would it be better to use pathlib for directory creation as well?

And shall we retain use of os.path methods where they are faster?

[The-Compiler]

Hi. Would it be better to use pathlib for directory creation as well?

I'd say so - pretty much anywhere.

And shall we retain use of os.path methods where they are faster?

Do you have a more concrete example?

[ju-sh]

Do you have a more concrete example?

The AsciiDoc class, inside scripts/asciidoc2html.py has an attribute _themedir where os.path.join() is used to find the value of the attribute _themedir of the AsciiDoc class inside scripts/asciidoc2html.py. (https://github.com/qutebrowser/qutebrowser/blob/master/scripts/asciidoc2html.py#L58)

If we convert _themedirto a pathlib.Path, we can use Path.joinpath() which seems to be a bit slower than os.path.join().

What do you think?

[The-Compiler]

I'd prefer using the / operator instead of the joinpath() method, i.e. self._themedir = self._homedir / '.asciidoc' / 'themes' / 'qute' for example.

Performance-wise we might need to think about it in cases where the whole application is affected (e.g. standarddir in qutebrowser), but for a script which takes some 10-20s anyways, a difference of 1µs or so doesn't matter at all.

[ju-sh]

Would it be good to add type hints? It would make stuff more readable and even be usable by mypy.

[The-Compiler]

Yep! I'd like to add more and more type hints over time (see #1456 and #5249), so that'd definitely be a welcome addition!

[ju-sh]

Modified asciidoc2html script. Couldn't figure out using type hints there though. Can you have a look?

#5290

[ju-sh]

What could be a suitable area to change next? Would changing tmpdir usages in tests to tmp_path be a good one?

[The-Compiler]

@ju-sh Yeah, that'd be great! Hard to estimate how much effort it'd be - tmpdir is used around 560 times (judging from a simple search with ag), but I suppose you could automate some of it. At least it's easy to tell whether something broke by running the tests!

Maybe it would make sense to split the work into multiple PRs though, since there's still some manual work involved. From a quick look at the usages, what do you think about opening separate PRs for:

• tests/unit/scripts
• tests/unit/misc
• tests/unit/utils
• tests/unit/browser
• tests/unit/config
• Rest of tests/unit/
• tests/end2end
• Anything remaining

That would help keeping the changes reasonably small and easy to review. Not sure how well it'll work out in practice, though.

Probably doesn't make much sense to add mypy type hints in the tests FWIW.

[ju-sh]

Hi. Here's some info that might be helpful while making changes related to this issue.

• os functions and pathlib equivalents: docs

• Change tmpdir fixture usages to tmp_path(docs)

• testdir fixture uses py.path but retain its use as a pathlib equivalent doesn't seem to exist.

py.path functions and their 'equivalents' (tmpdir fixture gives py.path):

py.path	equivalent
check()	Path.exists()
ensure()	Path.touch(exist_ok=True)
ensure(dir=True)	Path.mkdir(exist_ok=True)
size()	Path.stat().st_size

Hope this is useful. Please correct or augment if there's a mistake or anything to add.

[The-Compiler]

@ju-sh Thanks, that'll be helpful if others want to jump in as well! The only thing I have to add is that exist_ok=True might not be needed, depending on what the test is doing exactly.

@rbong @ju-sh Do you still intend to continue working on this, or is this something someone else could continue on?

[ju-sh]

@The-Compiler Sorry it's been so long and late..

I couldn't find enough time to keep working towards finishing this task on a regular basis.

Having more people working on this would be better.

[The-Compiler]

No worries, and thanks for getting the ball rolling!

[ghost]

If anyone else wants to help with this, I've replaced almost everything and have multiple branches for this on my fork

• I haven't touched the test/unit/helpers dir , (is the one containing fixtures.py,qutebrowser,)
• And one last file in qutebrowser/completion (specifically says that it uses os.path though I haven't look really if it was possible to find a replacement with pathlib)

I 'll also add type hints if I have the time (any help welcome )

[ghost]

Windows is just painful :

https://bugs.python.org/issue32434
https://bugs.python.org/issue36305
https://bugs.python.org/issue42855

You get inconsistent behaviour of pathlib.exists and resolve when you feed them with a path that does not actually exists