Remove use of importlib_resources throughout the codebase #707
Conversation
We can replace this with importlib.resources instead, now that we’re baselined on a high enough level of Python.
Doesn’t break anything as it stands as it overrides the earlier core_css_path_name, but we might as well match the process for the rest of the checks.
| @@ -182,7 +182,7 @@ def compare_versions(plain_output: bool) -> int: | |||
| for filename in natsorted(list(files_with_differences)): | |||
| html += f"\t\t<section>\n\t\t\t<h1>{filename.name}</h1>\n\t\t\t<img src=\"{filename.name}-original.png\">\n\t\t\t<img src=\"{filename.name}-new.png\">\n\t\t</section>\n" | |||
|
|
|||
| with importlib_resources.open_text("se.data.templates", "diff-template.html", encoding="utf-8") as file: | |||
| with importlib.resources.files("se.data.templates").joinpath("diff-template.html").open("r", encoding="utf-8") as file: | |||
There was a problem hiding this comment.
Can this accept a Path object instead of a string? If so then we should do that.
There was a problem hiding this comment.
I’ll investigate!
There was a problem hiding this comment.
So I think the answer’s no (thought this isn’t my forte): the files part creates an “Anchor” from the package, and joinpath then joins that with the given string to create a Path that can be opened.
If you think this is a bit wordy then we could cut it down with from importlib.resources import files, as_file and use them directly. Might need to import as though, files on its own is a bit generic.
There was a problem hiding this comment.
The docs say an anchor can be a string, so we should create a path using the Path object and then just cast it as a string when passed to .files(). So something like this should work (though I haven't tested):
with importlib.resources.files(str(Path("se.data.templates") / "diff-template.html")).open("r", encoding="utf-8") as file:The reason to try to do it this way is consistency, as the rest of the codebase uses Path and slashes, and not string concat or joinpath.
There was a problem hiding this comment.
I tested that in create_draft.py with the titlepage.svg generator, and it doesn’t work, ended up with:
ModuleNotFoundError: No module named 'se.data.templates/titlepage'
I think the problem is that these aren’t real Paths, they’re views into the package. Only when you get to the end is a Path produced for the rest of the system to interact with.
|
OK, I guess it doesn't work like I thought it did. Anyway this looks good, thanks! |
We can replace this with importlib.resources instead, now that we’re baselined on a high enough level of Python.
I’ve included a small commit that doesn’t have any functional change, but tidies up a piece of inconsistent linting code I noticed while doing this.
I’ll leave it up to you to do the other couple of setup.py changes, unless you would like me to add a commit. They’re: