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

Feature/better types #31

Merged
merged 19 commits into from
Sep 4, 2020
Merged

Feature/better types #31

merged 19 commits into from
Sep 4, 2020

Conversation

justindujardin
Copy link
Owner

@justindujardin justindujardin commented Sep 3, 2020

Refactor the codebase to get better types flowing throughout, and add lint check to the CI for making sure mypy is satisfied.

Changes

  • Add helpful error when using GCS paths without GCS libraries installed
  • Fix issue where GCS would print a client error immediately when importing Pathy
  • Autoformat and Lint check the codebase (sh tools/lint.sh / sh tools/format.sh)
  • Simplify GCS/File bucket client type names (e.g. ClientBucketGCS -> BucketGCS, ClientBlobFS -> BlobFS)
  • Consolidate the base pathy code in base.py removing api.py and client.py (needed to avoid circular imports)

Adds a simple registry of known schemes and their mappings to BucketClient subclasses. There's a hardcoded list of built-in services, and (in theory) you can register more dynamically.

I think I prefer to hardcode and include most of the known services, and lazily import them so you only need their packages when you actually use them. The hope is that this lets the strong typings flow through to the clients (because they can be statically inspected). If we can't get specific types flowing through nicely, maybe it's okay to do more of a dynamic import style registration.

BREAKING CHANGE use_fs, get_fs_client, use_fs_cache, get_fs_cache, and clear_fs_cache moved from pathy.api to pathy.clients
 - really the title is secondary to the commits in the PR. We'll continue to use the Semantic PR github app as long as it works
BREAKING CHANGE: This renames the internal GCS/File adapter classes by removing the prefix Client.

ClientBucketFS -> BucketFS
ClientBlobFS -> BlobFS
ClientBucketGCS -> BucketGCS
ClientBlobGCS -> BlobGCS
 - this makes the Pathy type accessible where it otherwise would not be for TypeVars.
 - in some cases the mypy errors are too uptight about subclasses and their types. When that happens we silence the error and provide the expected subclass type.
 - we black format last, so the order/indent could be changed.
 - since consolidating the Pathy class in the base.py file, there's no need to TypeVars, we can just use a forward ref to Pathy itself 🎉
@codecov
Copy link

codecov bot commented Sep 3, 2020

Codecov Report

Merging #31 into master will increase coverage by 1.10%.
The diff coverage is 84.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
+ Coverage   78.37%   79.47%   +1.10%     
==========================================
  Files           8        7       -1     
  Lines         823      872      +49     
==========================================
+ Hits          645      693      +48     
- Misses        178      179       +1     
Impacted Files Coverage Δ
pathy/about.py 0.00% <0.00%> (ø)
pathy/gcs.py 26.95% <35.82%> (-3.61%) ⬇️
pathy/cli.py 89.87% <75.00%> (-1.04%) ⬇️
pathy/base.py 90.97% <90.56%> (-2.37%) ⬇️
pathy/clients.py 93.10% <93.10%> (ø)
pathy/file.py 88.07% <94.59%> (+4.85%) ⬆️
pathy/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a78712...c7f6d19. Read the comment docs.

@justindujardin justindujardin added the enhancement New feature or request label Sep 4, 2020
@justindujardin justindujardin changed the base branch from fix/requirements to master September 4, 2020 00:36
@justindujardin justindujardin merged commit 51bf064 into master Sep 4, 2020
@justindujardin justindujardin deleted the feature/better-types branch September 4, 2020 00:41
@justindujardin
Copy link
Owner Author

🎉 This PR is included in version 0.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant