Skip to content

Enable tracking for dynamically defined TypeVar instances - #353

Merged
ogrisel merged 3 commits into
cloudpipe:masterfrom
ogrisel:dynamic-typevar-tracking
Mar 15, 2020
Merged

Enable tracking for dynamically defined TypeVar instances#353
ogrisel merged 3 commits into
cloudpipe:masterfrom
ogrisel:dynamic-typevar-tracking

Conversation

@ogrisel

Copy link
Copy Markdown
Contributor

Ensure that dynamic typevars are tracked and reconciled as dynamic classes and enums for consistency of physical equality semantics in cloudpickle.

Follow-up on #350 by @valtron, also discussed in #351.

@codecov

codecovBot commented Mar 15, 2020

Copy link
Copy Markdown

Codecov Report

Merging #353 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@## master #353 +/- ##
=======================================
Coverage 93.32% 93.33% =======================================
Files 2 2 Lines 764 765 +1 Branches 156 156 =======================================
+ Hits 713 714 +1 
Misses 26 26 Partials 25 25 
Impacted FilesCoverage Δ
cloudpickle/cloudpickle_fast.py95.72% <ø> (ø)
cloudpickle/cloudpickle.py92.27% <100.00%> (+0.01%)⬆️

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 3e80b26...abbd042. Read the comment docs.

@ogrisel

Copy link
Copy Markdown
ContributorAuthor

I checked that the new test did not pass prior to making the change.

@ogrisel

Copy link
Copy Markdown
ContributorAuthor

I also include a renaming of _ensure_tracking to _get_or_create_tracker_id to make the code easier to follow by being more explicit about what this function returns.

@ogrisel
ogrisel merged commit d8452cc into cloudpipe:masterMar 15, 2020
@ogrisel
ogrisel deleted the dynamic-typevar-tracking branch March 15, 2020 19:59
@ogrisel

Copy link
Copy Markdown
ContributorAuthor

@pierreglaser what do you think of making a release of cloudpickle to ship type annotation support?

@pierreglaser

Copy link
Copy Markdown
Member

Sure, this deserves a minor relase on its own. We'll include #357 in the next bugfix release?

@ogrisel

Copy link
Copy Markdown
ContributorAuthor

We'll include #357 in the next bugfix release?

+1 for dealing with the #357 in another release no to delay the shipping of the new type annotation support.

Sign up for freeto 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.

2 participants

@ogrisel@pierreglaser