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.

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

Replace usage of os.path and path.py with pathlib #225

Merged
merged 3 commits into from
Apr 30, 2024
Merged

Replace usage of os.path and path.py with pathlib #225

merged 3 commits into from
Apr 30, 2024

Conversation

elfkuzco
Copy link
Contributor

@elfkuzco elfkuzco commented Apr 13, 2024

Fixes #195

Rationale

Use pathlib.Path for all path operations instead of a combination of os.path and path.py.

Changes

  • Replace all usages of os.path and path.py to pathlib.Path
  • Add type annotations for some parts of the code
  • Use f-strings to build urls instead of os.path.join

Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

Thank you, this looks very good, only minor comments to discuss (not sure I'm right on all of them ^^)

src/gutenberg2zim/download.py Show resolved Hide resolved
src/gutenberg2zim/entrypoint.py Outdated Show resolved Hide resolved
src/gutenberg2zim/export.py Outdated Show resolved Hide resolved
src/gutenberg2zim/download.py Outdated Show resolved Hide resolved
src/gutenberg2zim/download.py Outdated Show resolved Hide resolved
src/gutenberg2zim/rdf.py Outdated Show resolved Hide resolved
src/gutenberg2zim/urls.py Outdated Show resolved Hide resolved
@elfkuzco
Copy link
Contributor Author

@benoit74 , I have made the necessary fixes but I'm unable to run the full check anymore as my VM instance on GCP keeps running out of space (over 20Gb). However, I can verify that the previous commands run to full completion

@benoit74
Copy link
Collaborator

I have made the necessary fixes but I'm unable to run the full check anymore as my VM instance on GCP keeps running out of space (over 20Gb). However, I can verify that the previous commands run to full completion

That's enough for me, we will make a full run before releasing this anyway.

@benoit74 benoit74 self-requested a review April 30, 2024 11:01
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

LGTM

@benoit74 benoit74 merged commit f344086 into openzim:main Apr 30, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove usage of os.path and path.py, prefer pathlib
2 participants