Skip to content

Typecheck and test improvements #345

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

Merged
merged 5 commits into from
Mar 21, 2020

Conversation

pcattori
Copy link
Contributor

@pcattori pcattori commented Mar 21, 2020

↪️ Pull Request

This PR fixes typechecking for the tamr_client package (and corresponding tests). Previously, some typechecking errors went unnoticed by mypy due to known mypy --package bugs (see python/mypy#5759 ).

Also, move all tamr_client tests out of the global tests namespace and into their own top-level namespace: tests/tamr_client.

Finally, move BETA check into a function in its own module so that tamr_client.__init__.py does not have extraneous imports/code that users who are autocompleting tc. could erroneously think are part of the intended interface.

✔️ PR Todo

  • [N/A] Added/updated testing for this change
  • [N/A] Included links to related issues/PRs
  • [N/A] Update relevant docs + docstrings
  • [N/A] Update the CHANGELOG under the current -dev version:
    • Add changelog entries under any that apply: BREAKING CHANGES, NEW FEATURES, BUG FIXES.
    • Changelog entry format: [#<issue number>](<link to issue>) <change description>

Before it was confusing/unclear that some directories were for testing
the tamr_client package and others for tamr_unify_client package.
Packages should be typechecked with `--package` flag. Previously, a
typecheck error in attribute.py went unnoticed by mypy.

Additionally, for `--package` to work, __init__.py files are required
(at least for now, see python/mypy#5759 ).
...should return a tuple of 1 *or more* attributes. The correct syntax
for this is `Tuple[<type>, ...]` not `Tuple[<type>]`
Users autocompleting `tc.` won't get results like `tc.os` like before
since that `os` import was for the BETA check that is now in a separate
module.

Also, reorder tamr_client/__init__.py with comment headers for clarity.
@pcattori
Copy link
Contributor Author

@skalish fyi: I moved tamr_client tests into tests/tamr_client. Could affect your PR #339 so you may need to git checkout master, git pull Datatamer master and then git checkout <branch>, git rebase master

@pcattori pcattori merged commit 6f26029 into Datatamer:master Mar 21, 2020
@pcattori pcattori deleted the typecheck-and-test-improvements branch March 21, 2020 00:28
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.

1 participant