CQ: Fix Ruff PTH113: os.path.isfile() should be replaced by Path.is_file()

[echoix]

Ruff rule: https://docs.astral.sh/ruff/rules/os-path-isfile/

I've also fixed the other PTH ruff rules in the same lines changed, when appropriate, so there's a little more than only PTH113 fixed.

For tests, I mostly agreed on the simplification of using pathlib.Path.unlink with missing_ok=True instead of checking if it is a file. The only difference, is that using Path("something").unlink(missing_ok=True) will throw an IsADirectoryError. But in the context of a test, where the file to delete is a class property, I accepted that.

In [1]: from pathlib import Path

In [2]: b = Path("test123")

In [3]: b.unlink()
---------------------------------------------------------------------------
IsADirectoryError                         Traceback (most recent call last)
Cell In[3], line 1
----> 1 b.unlink()

File /usr/local/python/3.12.7/lib/python3.12/pathlib.py:1342, in Path.unlink(self, missing_ok)
   1337 """
   1338 Remove this file or link.
   1339 If the path is a directory, use rmdir() instead.
   1340 """
   1341 try:
-> 1342     os.unlink(self)
   1343 except FileNotFoundError:
   1344     if not missing_ok:

IsADirectoryError: [Errno 21] Is a directory: 'test123'

In [4]: 

[ninsbl]

Nice. Again, I learned something from reviewing (have not seen or used (p := Path(mfile)) before.

Just some minor suggestions where I think os.path.join inside Path initialisation is not needed...

[wenzeslaus]

I had issues recently with Path.exists that it was behaving differently than os.path equivalent when permissions were read-only. At least one Linux, calling Path.exists() raised permission denied error, but os.path did not where parent directory was read-only for a file which does not exist. I found that when testing pre-conditions in an r.pack test. That's just to say that Path or os.path may not be 100% the same, although I assume that's the intention and the general direction where the library is moving, so I don't know whether to take any action here.

@pytest.mark.skipif(sys.platform.startswith("win"), reason="Dir still writable")
def test_output_dir_is_not_writable(xy_raster_dataset_session_for_module, tmp_path):
    """Check behavior when directory is not writable"""
    tools = Tools(session=xy_raster_dataset_session_for_module)
    parent = tmp_path / "parent_dir"
    parent.mkdir()
    # This should work for Windows according to the doc, but it does not.
    parent.chmod(stat.S_IREAD)
    output = parent / "output_1.rpack"
    # Calling output.exists() gives permission denied on Linux, but os.path does not.
    assert not os.path.exists(output)  # noqa: PTH110
    assert output.parent.exists()
    assert output.parent.is_dir()
    with pytest.raises(
        CalledModuleError,
        match=rf"(?s)'{re.escape(str(output.parent))}'.*is not.*writable",
    ):
        tools.r_pack(input="rows_raster", output=output)

[echoix]

I had issues recently with Path.exists that it was behaving differently than os.path equivalent when permissions were read-only. At least one Linux, calling Path.exists() raised permission denied error, but os.path did not where parent directory was read-only for a file which does not exist. I found that when testing pre-conditions in an r.pack test. That's just to say that Path or os.path may not be 100% the same, although I assume that's the intention and the general direction where the library is moving, so I don't know whether to take any action here.

@pytest.mark.skipif(sys.platform.startswith("win"), reason="Dir still writable")
def test_output_dir_is_not_writable(xy_raster_dataset_session_for_module, tmp_path):
    """Check behavior when directory is not writable"""
    tools = Tools(session=xy_raster_dataset_session_for_module)
    parent = tmp_path / "parent_dir"
    parent.mkdir()
    # This should work for Windows according to the doc, but it does not.
    parent.chmod(stat.S_IREAD)
    output = parent / "output_1.rpack"
    # Calling output.exists() gives permission denied on Linux, but os.path does not.
    assert not os.path.exists(output)  # noqa: PTH110
    assert output.parent.exists()
    assert output.parent.is_dir()
    with pytest.raises(
        CalledModuleError,
        match=rf"(?s)'{re.escape(str(output.parent))}'.*is not.*writable",
    ):
        tools.r_pack(input="rows_raster", output=output)

I also remember a place where we (my PR and @ninsbl reviewing) that we had to revert a change in a r.pack test.

If we encounter such exceptional cases, or if we optimise enough to find that it becomes a problem to use path lib objects instead of a string, then these will be good cases to ignore such suggestions and keep using os.path.

[echoix]

I had a follow up PR made the same day last week but not opened yet, because it would need a rebase on top of it (continuing the changes from here). And the original work started from that PR was the week before