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

Static Typing profilers/profiler_options.py #644

Merged

Conversation

tonywu315
Copy link
Contributor

No description provided.

@tonywu315
Copy link
Contributor Author

#608

@taylorfturner taylorfturner added the static_typing mypy static typing issues label Sep 19, 2022
@@ -1,9 +1,12 @@
#!/usr/bin/env python
"""Specify the options when running the data profiler."""
from __future__ import annotations
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On line 36, I added

if TYPE_CHECKING:
    from .profile_builder import BaseProfiler

because BaseProfiler has a circular dependency with utils. Adding this means that I need to postpone the evaluation of type annotations, since BaseProfiler is not defined until later. In Python 3.7, this can be done by importing annotations from __future__ or by using "BaseProfiler" instead of BaseProfiler for types.

Copy link
Contributor Author

@tonywu315 tonywu315 Sep 19, 2022

Choose a reason for hiding this comment

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

Oh I thought this was for the utils PR. For this one, it's because the class types aren't defined until later. PEP 563

@JGSweets
Copy link
Collaborator

@tonywu315 FYI, I think your code is out of date. You will likely need to rebase this branch onto C1/main.

1 similar comment
@JGSweets
Copy link
Collaborator

@tonywu315 FYI, I think your code is out of date. You will likely need to rebase this branch onto C1/main.

@JGSweets JGSweets enabled auto-merge (squash) September 20, 2022 16:14
@JGSweets JGSweets merged commit 904bd3e into capitalone:main Sep 20, 2022
@tonywu315 tonywu315 deleted the static_typing/profilers/profiler_options branch September 20, 2022 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
static_typing mypy static typing issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants