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

fix(typing): update types to be compatible with latest mypy #4234

Merged
merged 6 commits into from
Sep 27, 2022

Conversation

Yun-Kim
Copy link
Contributor

@Yun-Kim Yun-Kim commented Sep 26, 2022

Description

The latest 0.981 release of mypy which our CI checks and commit hooks are dependent on resulted in previously uncaught errors. This new release of mypy includes new specifications and rules for TypeVars and abstract classes, which has been addressed in this PR.

Note: there's an ongoing issue (which I've had to ignore for now to unblock CI) with our from_env() helper in ddtrace.internal.utils.attr.py where mypy is expecting either a Type (i.e. str, float, int,...) or a Callable (i.e. formats.asbool). However, mypy mistakes the Type as a Callable and results in errors in the mypy type checking.

The current hypothesis is that since the types mentioned above are actually callables, (i.e. float(), str(), int(),...), mypy is mistaking the Type as a callable and causing the type checking error. I'm going to try to reproduce this, then if it persists create an issue on mypy to see what we can do to resolve this.

Update: created an issue on mypy. Looks like this is currently a known issue without much workaround. :(

@Yun-Kim Yun-Kim added the changelog/no-changelog A changelog entry is not required for this PR. label Sep 26, 2022
@Yun-Kim Yun-Kim requested a review from a team as a code owner September 26, 2022 21:24
@Kyle-Verhoog Kyle-Verhoog changed the title chore[ci]: Fix typing with upgraded mypy chore(ci): Fix typing with upgraded mypy Sep 26, 2022
Kyle-Verhoog
Kyle-Verhoog previously approved these changes Sep 26, 2022
@Kyle-Verhoog Kyle-Verhoog changed the title chore(ci): Fix typing with upgraded mypy fix(typing): update types to be compatible with latest mypy Sep 26, 2022
@Yun-Kim
Copy link
Contributor Author

Yun-Kim commented Sep 26, 2022

Note: there's an issue with our from_env() helper in ddtrace.internal.utils.attr.py where mypy is expecting either a Type (i.e. str, float, int,...) or a Callable (i.e. formats.asbool). However, mypy mistakes the Type as a Callable and results in errors in the mypy type checking.

The current hypothesis is that since the types mentioned above are actually callables, (i.e. float(), str(), int(),...), mypy is mistaking the Type as a callable and causing the type checking error.

avara1986
avara1986 previously approved these changes Sep 27, 2022
@P403n1x87 P403n1x87 added the manual merge Do not automatically merge label Sep 27, 2022
@Yun-Kim Yun-Kim dismissed stale reviews from avara1986 and Kyle-Verhoog via 872ff13 September 27, 2022 14:05
Copy link
Contributor

@P403n1x87 P403n1x87 left a comment

Choose a reason for hiding this comment

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

Just one small change, otherwise LGTM!

ddtrace/internal/utils/cache.py Outdated Show resolved Hide resolved
Copy link
Contributor

@P403n1x87 P403n1x87 left a comment

Choose a reason for hiding this comment

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

💪

@brettlangdon brettlangdon merged commit 5140de4 into DataDog:1.x Sep 27, 2022
@Yun-Kim Yun-Kim deleted the yunkim/mypy_check branch September 27, 2022 16:12
@Yun-Kim Yun-Kim mentioned this pull request Oct 3, 2022
14 tasks
Kyle-Verhoog pushed a commit that referenced this pull request Oct 3, 2022
Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/__init__.py
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/__init__.py
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py
@Yun-Kim
Copy link
Contributor Author

Yun-Kim commented Oct 3, 2022

@Mergifyio backport 1.5 1.4 0.61 0.x

mergify bot pushed a commit that referenced this pull request Oct 3, 2022
* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

# Conflicts:
#	ddtrace/debugging/_debugger.py
#	ddtrace/internal/utils/cache.py
#	ddtrace/profiling/collector/__init__.py
#	ddtrace/profiling/collector/asyncio.py
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

# Conflicts:
#	ddtrace/debugging/_debugger.py
#	ddtrace/internal/utils/cache.py
#	ddtrace/profiling/collector/__init__.py
#	ddtrace/profiling/collector/asyncio.py
@mergify
Copy link
Contributor

mergify bot commented Oct 3, 2022

mabdinur added a commit that referenced this pull request Oct 4, 2022
* Remove ignores for updated mypy (#4253)

Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py

* fix conflicts

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>
brettlangdon added a commit that referenced this pull request Oct 4, 2022
…4234) (#4263)

* fix(typing): update types to be compatible with latest mypy (#4234)

* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

* ci(mypy): remove type callable ignores (backport #4253) (#4258)

* Remove ignores for updated mypy (#4253)

Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py

* fix conflicts

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>
mergify bot added a commit that referenced this pull request Oct 4, 2022
…4234) (#4263)

* fix(typing): update types to be compatible with latest mypy (#4234)

* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

* ci(mypy): remove type callable ignores (backport #4253) (#4258)

* Remove ignores for updated mypy (#4253)

Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py

* fix conflicts

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>
(cherry picked from commit 026d614)
mergify bot added a commit that referenced this pull request Oct 4, 2022
…4234) (#4263)

* fix(typing): update types to be compatible with latest mypy (#4234)

* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

* ci(mypy): remove type callable ignores (backport #4253) (#4258)

* Remove ignores for updated mypy (#4253)

Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py

* fix conflicts

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>
(cherry picked from commit 026d614)

# Conflicts:
#	ddtrace/debugging/_debugger.py
#	ddtrace/internal/utils/cache.py
#	ddtrace/profiling/collector/asyncio.py
mergify bot added a commit that referenced this pull request Oct 4, 2022
…4234) (#4263)

* fix(typing): update types to be compatible with latest mypy (#4234)

* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

* ci(mypy): remove type callable ignores (backport #4253) (#4258)

* Remove ignores for updated mypy (#4253)

Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py

* fix conflicts

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>
(cherry picked from commit 026d614)

# Conflicts:
#	ddtrace/debugging/_debugger.py
#	ddtrace/internal/utils/cache.py
#	ddtrace/profiling/collector/asyncio.py
brettlangdon pushed a commit that referenced this pull request Oct 4, 2022
…4234) (#4263) (#4276)

* fix(typing): update types to be compatible with latest mypy (#4234)

* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

* ci(mypy): remove type callable ignores (backport #4253) (#4258)

* Remove ignores for updated mypy (#4253)

Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py

* fix conflicts

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>
(cherry picked from commit 026d614)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
brettlangdon pushed a commit that referenced this pull request Oct 4, 2022
…4234) (backport #4263) (#4278)

* fix(typing): update types to be compatible with latest mypy (backport #4234) (#4263)

* fix(typing): update types to be compatible with latest mypy (#4234)

* Initial mypy fixes for CI

* Ignored mypy error with type and type constructors

* Fix formatting

* Fixed incorrect typing

* Undo initial change

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
(cherry picked from commit 5140de4)

* ci(mypy): remove type callable ignores (backport #4253) (#4258)

* Remove ignores for updated mypy (#4253)

Mypy==0.982 has just been released and fixed the inherent type/callable type hinting that was the problem from #4234. As a result, the type ignore comments added in #4234 to bypass the type/callable issue are not unneeded and are breaking CI.

(cherry picked from commit 0ba3e29)

# Conflicts:
#	ddtrace/profiling/collector/memalloc.py
#	ddtrace/profiling/exporter/http.py

* fix conflicts

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>

Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com>
(cherry picked from commit 026d614)

# Conflicts:
#	ddtrace/debugging/_debugger.py
#	ddtrace/internal/utils/cache.py
#	ddtrace/profiling/collector/asyncio.py

* Resolve merge conflicts with 0.61

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Yun Kim <yun.kim@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog A changelog entry is not required for this PR. manual merge Do not automatically merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants