Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove use of importlib_resources throughout the codebase #707

Merged
merged 2 commits into from
Jun 10, 2024
Merged

Remove use of importlib_resources throughout the codebase #707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3397bc
Remove use of importlib_resources throughout the codebase
robinwhittleton Jun 9, 2024
15aa6be
Make se.css linting path variable unique
robinwhittleton Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mypy.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ warn_unused_ignores = True
[mypy-se.vendor.*]
ignore_errors = True

[mypy-importlib_resources,pytest,regex,lxml.*,smartypants,hyphen.*,git,roman,tinycss2.*,cssselect,pyphen,titlecase,magic,ftfy,cairosvg,selenium.*,PIL,natsort,cssutils,psutil,chardet,rich.*]
[mypy-pytest,regex,lxml.*,smartypants,hyphen.*,git,roman,tinycss2.*,cssselect,pyphen,titlecase,magic,ftfy,cairosvg,selenium.*,PIL,natsort,cssutils,psutil,chardet,rich.*]
ignore_missing_imports = True
2 changes: 1 addition & 1 deletion pylintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ignore=vendor
extension-pkg-whitelist=lxml,math,unicodedata

[MESSAGES CONTROL]
disable=line-too-long,too-many-nested-blocks,too-many-branches,too-many-statements,too-many-boolean-expressions,too-many-lines,too-many-locals,broad-except,too-few-public-methods,too-many-arguments,too-many-instance-attributes,too-many-public-methods,duplicate-code,deprecated-method
disable=line-too-long,too-many-nested-blocks,too-many-branches,too-many-statements,too-many-boolean-expressions,too-many-lines,too-many-locals,broad-except,too-few-public-methods,too-many-arguments,too-many-instance-attributes,too-many-public-methods,duplicate-code

[FORMAT]
indent-string=\t
Expand Down
4 changes: 2 additions & 2 deletions se/commands/compare_versions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import shutil
import tempfile
from pathlib import Path
import importlib.resources

import importlib_resources
import git
from natsort import natsorted
from PIL import Image, ImageChops
Expand Down Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this accept a Path object instead of a string? If so then we should do that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll investigate!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

html = file.read().replace("<!--se:sections-->", html.strip())

with open(output_directory / "diff.html", "w", encoding="utf-8") as file:
Expand Down
8 changes: 4 additions & 4 deletions se/commands/create_draft.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
from argparse import Namespace
from html import escape
from pathlib import Path
import importlib.resources
from typing import Optional, Tuple, Union, List, Dict

import git
import importlib_resources
import regex
import requests
from rich.console import Console
Expand Down Expand Up @@ -190,7 +190,7 @@ def _generate_titlepage_svg(title: str, authors: List[str], contributors: dict,
canvas_width = se.TITLEPAGE_WIDTH - (TITLEPAGE_HORIZONTAL_PADDING * 2)

# Read our template SVG to get some values before we begin
with importlib_resources.open_text("se.data.templates", "titlepage.svg", encoding="utf-8") as file:
with importlib.resources.files("se.data.templates").joinpath("titlepage.svg").open("r", encoding="utf-8") as file:
svg = file.read()

# Remove the template text elements from the SVG source, we'll write out to it later
Expand Down Expand Up @@ -286,7 +286,7 @@ def _generate_cover_svg(title: str, authors: List[str], title_string: str) -> st
canvas_width = COVER_TITLE_BOX_WIDTH - (COVER_TITLE_BOX_PADDING * 2)

# Read our template SVG to get some values before we begin
with importlib_resources.open_text("se.data.templates", "cover.svg", encoding="utf-8") as file:
with importlib.resources.files("se.data.templates").joinpath("cover.svg").open("r", encoding="utf-8") as file:
svg = file.read()

# Remove the template text elements from the SVG source, we'll write out to it later
Expand Down Expand Up @@ -406,7 +406,7 @@ def _copy_template_file(filename: str, dest_path: Path) -> None:
"""
if dest_path.is_dir():
dest_path = dest_path / filename
with importlib_resources.path("se.data.templates", filename) as src_path:
with importlib.resources.as_file(importlib.resources.files("se.data.templates").joinpath(filename)) as src_path:
shutil.copyfile(src_path, dest_path)

def _add_name_abbr(contributor: str) -> str:
Expand Down
4 changes: 2 additions & 2 deletions se/commands/split_file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import argparse
from pathlib import Path
import importlib.resources

import importlib_resources
import regex
import roman

Expand Down Expand Up @@ -57,7 +57,7 @@ def split_file(plain_output: bool) -> int:
se.print_error(f"Couldn’t open file: [path][link=file://{filename}]{filename}[/][/].", plain_output=plain_output)
return se.InvalidFileException.code
else:
with importlib_resources.open_text("se.data.templates", "chapter-template.xhtml", encoding="utf-8") as file:
with importlib.resources.files("se.data.templates").joinpath("chapter-template.xhtml").open("r", encoding="utf-8") as file:
template_xhtml = file.read()

# Try to guess the ebook language and update the template accordingly
Expand Down
4 changes: 2 additions & 2 deletions se/images.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
import tempfile
import struct
import urllib.parse
import importlib.resources

from html import unescape
from typing import List, Callable, Dict
import regex
from PIL import Image, ImageMath, PngImagePlugin, UnidentifiedImageError
from PIL.Image import Image as Image_type # Separate import to satisfy type checking
import importlib_resources
from lxml import etree

import se
Expand Down Expand Up @@ -313,7 +313,7 @@ def svg_text_to_paths(in_svg: Path, out_svg: Path, remove_style=True) -> None:
name_list = {"league_spartan": ["league-spartan-bold.svg"], "sorts_mill_goudy": ["sorts-mill-goudy-italic.svg", "sorts-mill-goudy.svg"]}
for font_family, font_names in name_list.items():
for font_name in font_names:
with importlib_resources.path(f"se.data.fonts.{font_family}", font_name) as font_path:
with importlib.resources.as_file(importlib.resources.files(f"se.data.fonts.{font_family}").joinpath(font_name)) as font_path:
font_paths.append(font_path)
fonts = []
for font_path in font_paths:
Expand Down
18 changes: 9 additions & 9 deletions se/se_epub_build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
from hashlib import sha1
from html import unescape
from pathlib import Path
import importlib.resources
from typing import Dict, Tuple, List, Optional
import importlib_resources

from cairosvg import svg2png
from PIL import Image, ImageOps
Expand Down Expand Up @@ -179,7 +179,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k
# Are we including proofreading CSS?
if proof:
with open(work_compatible_epub_dir / "epub" / "css" / "local.css", "a", encoding="utf-8") as local_css_file:
with importlib_resources.open_text("se.data.templates", "proofreading.css", encoding="utf-8") as proofreading_css_file:
with importlib.resources.files("se.data.templates").joinpath("proofreading.css").open("r", encoding="utf-8") as proofreading_css_file:
local_css_file.write("\n" + proofreading_css_file.read())

# Update the release date in the metadata and colophon
Expand Down Expand Up @@ -227,7 +227,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k
compatibility_css_filename = "compatibility-white-label.css"

with open(work_compatible_epub_dir / "epub" / "css" / "core.css", "a", encoding="utf-8") as core_css_file:
with importlib_resources.open_text("se.data.templates", compatibility_css_filename, encoding="utf-8") as compatibility_css_file:
with importlib.resources.files("se.data.templates").joinpath(compatibility_css_filename).open("r", encoding="utf-8") as compatibility_css_file:
core_css_file.write("\n" + compatibility_css_file.read())

# Simplify CSS and tags
Expand Down Expand Up @@ -430,7 +430,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k

# Initialize the transform object, if we haven't yet
if not mathml_transform:
with importlib_resources.path("se.data", "mathmlcontent2presentation.xsl") as mathml_xsl_filename:
with importlib.resources.as_file(importlib.resources.files("se.data").joinpath("mathmlcontent2presentation.xsl")) as mathml_xsl_filename:
mathml_transform = etree.XSLT(etree.parse(str(mathml_xsl_filename)))

# Transform the mathml and get a string representation
Expand Down Expand Up @@ -1064,7 +1064,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k
node.append(etree.fromstring("""<item href="toc.ncx" id="ncx" media-type="application/x-dtbncx+xml"/>"""))

# Now use an XSLT transform to generate the NCX
with importlib_resources.path("se.data", "navdoc2ncx.xsl") as navdoc2ncx_xsl_filename:
with importlib.resources.as_file(importlib.resources.files("se.data").joinpath("navdoc2ncx.xsl")) as navdoc2ncx_xsl_filename:
toc_tree = se.epub.convert_toc_to_ncx(work_compatible_epub_dir, toc_filename, navdoc2ncx_xsl_filename)

# Convert the <nav> landmarks element to the <guide> element in the metadata file
Expand Down Expand Up @@ -1135,7 +1135,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k

if run_epubcheck:
# Path arguments must be cast to string for Windows compatibility.
with importlib_resources.path("se.data.epubcheck", "epubcheck.jar") as jar_path:
with importlib.resources.as_file(importlib.resources.files("se.data.epubcheck").joinpath("epubcheck.jar")) as jar_path:
# We have to use a temp file to hold stdout, because if the output is too large for the output buffer in subprocess.run() (and thus popen()) it will be truncated
with tempfile.TemporaryFile() as stdout:
# We can't check the return code, because if only warnings are returned then epubcheck will return 0 (success)
Expand Down Expand Up @@ -1169,7 +1169,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k
raise se.BuildFailedException("[bash]epubcheck[/] failed.", build_messages)

# Now run the Nu Validator
with importlib_resources.path("se.data.vnu", "vnu.jar") as jar_path:
with importlib.resources.as_file(importlib.resources.files("se.data.vnu").joinpath("vnu.jar")) as jar_path:
# We have to use a temp file to hold stdout, because if the output is too large for the output buffer in subprocess.run() (and thus popen()) it will be truncated
with tempfile.TemporaryFile() as stdout:
subprocess.run(["java", "-jar", str(jar_path), "--format", "xml", str(self.epub_root_path / "epub" / "text")], stdout=stdout, stderr=stdout, check=False)
Expand Down Expand Up @@ -1312,7 +1312,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k
file.truncate()

# Rebuild the NCX
with importlib_resources.path("se.data", "navdoc2ncx.xsl") as navdoc2ncx_xsl_filename:
with importlib.resources.as_file(importlib.resources.files("se.data").joinpath("navdoc2ncx.xsl")) as navdoc2ncx_xsl_filename:
se.epub.convert_toc_to_ncx(work_compatible_epub_dir, toc_filename, navdoc2ncx_xsl_filename)

# Clean just the ToC and NCX
Expand Down Expand Up @@ -1405,7 +1405,7 @@ def build(self, run_epubcheck: bool, check_only: bool, build_kobo: bool, build_k

# Include compatibility CSS
with open(work_compatible_epub_dir / "epub" / "css" / "core.css", "a", encoding="utf-8") as core_css_file:
with importlib_resources.open_text("se.data.templates", "kindle.css", encoding="utf-8") as compatibility_css_file:
with importlib.resources.files("se.data.templates").joinpath("kindle.css").open("r", encoding="utf-8") as compatibility_css_file:
core_css_file.write("\n" + compatibility_css_file.read())

# Build an epub file we can send to Calibre
Expand Down
20 changes: 10 additions & 10 deletions se/se_epub_lint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import os
from pathlib import Path
from typing import Dict, List, Set, Union, Optional
import importlib_resources
import importlib.resources
from unidecode import unidecode

import cssutils
Expand Down Expand Up @@ -161,7 +161,7 @@
"f-011", "JPEG files must end in [path].jpg[/]."
"f-012", "TIFF files must end in [path].tif[/]."
"f-013", "Glossary search key map must be named [path]glossary-search-key-map.xml[/]."
"f-014", f"File does not match [path][link=file://{self.path / 'src/epub/css/se.css'}]{core_css_file_path}[/][/]."
"f-014", f"File does not match [path][link=file://{se_css_file_path}]{se_css_file_path}[/][/]."
"f-015", "Filename doesn’t match [attr]id[/] attribute of primary [xhtml]<section>[/] or [xhtml]<article>[/]. Hint: [attr]id[/] attributes don’t include the file extension."
"f-016", "Image more than 1.5MB in size."
"f-017", f"[path][link=file://{self.path / 'images/cover.jpg'}]cover.jpg[/][/] must be exactly {se.COVER_WIDTH} × {se.COVER_HEIGHT}."
Expand Down Expand Up @@ -3259,7 +3259,7 @@ def lint(self, skip_lint_ignore: bool, allowed_messages: Optional[List[str]] = N
}

# Cache the browser default stylesheet for later use
with importlib_resources.open_text("se.data", "browser.css", encoding="utf-8") as css:
with importlib.resources.files("se.data").joinpath("browser.css").open("r", encoding="utf-8") as css:
self._file_cache["default"] = css.read() # pylint: disable=protected-access

# Check that the spine has all the expected files in the book
Expand Down Expand Up @@ -3386,37 +3386,37 @@ def lint(self, skip_lint_ignore: bool, allowed_messages: Optional[List[str]] = N
# Make sure some static files are unchanged
if self.is_se_ebook:
try:
with importlib_resources.path("se.data.templates", "LICENSE.md") as license_file_path:
with importlib.resources.as_file(importlib.resources.files("se.data.templates").joinpath("LICENSE.md")) as license_file_path:
if not filecmp.cmp(license_file_path, self.path / "LICENSE.md"):
messages.append(LintMessage("f-003", f"File does not match [path][link=file://{license_file_path}]{license_file_path}[/][/].", se.MESSAGE_TYPE_ERROR, self.path / "LICENSE.md"))
except Exception:
missing_files.append("LICENSE.md")

try:
with importlib_resources.path("se.data.templates", "core.css") as core_css_file_path:
with importlib.resources.as_file(importlib.resources.files("se.data.templates").joinpath("core.css")) as core_css_file_path:
if not filecmp.cmp(core_css_file_path, self.content_path / "css/core.css"):
messages.append(LintMessage("f-004", f"File does not match [path][link=file://{core_css_file_path}]{core_css_file_path}[/][/].", se.MESSAGE_TYPE_ERROR, self.content_path / "css/core.css"))
except Exception:
missing_files.append("css/core.css")

try:
with importlib_resources.path("se.data.templates", "logo.svg") as logo_svg_file_path:
with importlib.resources.as_file(importlib.resources.files("se.data.templates").joinpath("logo.svg")) as logo_svg_file_path:
if not filecmp.cmp(logo_svg_file_path, self.content_path / "images/logo.svg"):
messages.append(LintMessage("f-005", f"File does not match [path][link=file://{logo_svg_file_path}]{logo_svg_file_path}[/][/].", se.MESSAGE_TYPE_ERROR, self.content_path / "images/logo.svg"))
except Exception:
missing_files.append("images/logo.svg")

try:
with importlib_resources.path("se.data.templates", "uncopyright.xhtml") as uncopyright_file_path:
with importlib.resources.as_file(importlib.resources.files("se.data.templates").joinpath("uncopyright.xhtml")) as uncopyright_file_path:
if not filecmp.cmp(uncopyright_file_path, self.content_path / "text/uncopyright.xhtml"):
messages.append(LintMessage("f-006", f"File does not match [path][link=file://{uncopyright_file_path}]{uncopyright_file_path}[/][/].", se.MESSAGE_TYPE_ERROR, self.content_path / "text/uncopyright.xhtml"))
except Exception:
missing_files.append("text/uncopyright.xhtml")

try:
with importlib_resources.path("se.data.templates", "se.css") as core_css_file_path:
if not filecmp.cmp(core_css_file_path, self.content_path / "css/se.css"):
messages.append(LintMessage("f-014", f"File does not match [path][link=file://{self.path / 'src/epub/css/se.css'}]{core_css_file_path}[/][/].", se.MESSAGE_TYPE_ERROR, self.content_path / "css/se.css"))
with importlib.resources.as_file(importlib.resources.files("se.data.templates").joinpath("se.css")) as se_css_file_path:
if not filecmp.cmp(se_css_file_path, self.content_path / "css/se.css"):
messages.append(LintMessage("f-014", f"File does not match [path][link=file://{se_css_file_path}]{se_css_file_path}[/][/].", se.MESSAGE_TYPE_ERROR, self.content_path / "css/se.css"))
except Exception:
missing_files.append("css/se.css")

Expand Down
4 changes: 2 additions & 2 deletions se/spelling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"""

from typing import Set
import importlib_resources
import importlib.resources
import regex
import se

Expand Down Expand Up @@ -35,7 +35,7 @@ def initialize_dictionary():
"""

if not se.spelling.DICTIONARY:
with importlib_resources.open_text("se.data", "words") as dictionary:
with importlib.resources.files("se.data").joinpath("words").open("r", encoding="utf-8") as dictionary:
se.spelling.DICTIONARY = {line.strip().lower() for line in dictionary}

def modernize_hyphenation(xhtml: str) -> str:
Expand Down
Loading