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

Implement cache for DelegateParameter to mirror cache of the source parameter #1827

Merged
merged 20 commits into from
Nov 22, 2019

Conversation

Dominik-Vogel
Copy link
Contributor

@Dominik-Vogel Dominik-Vogel commented Nov 14, 2019

This PR implements cache of DelegateParameter to exactly mirror the cache of the source parameter. Depends on #1832.

For now, some dirty stuff with respect to accessing _value of cache is needed, but this wouldn't be necessary when snapshot_base improvements will be worked on in #1833.

Fixes #1782.

@astafan8
Copy link
Contributor

@Dominik-Vogel could you please rebase this PR on top of #1832 ?

@Dominik-Vogel
Copy link
Contributor Author

done

@Dominik-Vogel
Copy link
Contributor Author

(Rebased onto wrong branch, really sorry, I lost the overview over all the different cache PRs)
Now it is updated and sound, @astafan8 .

@codecov
Copy link

codecov bot commented Nov 21, 2019

Codecov Report

Merging #1827 into master will increase coverage by 0.03%.
The diff coverage is 92.59%.

@@            Coverage Diff             @@
##           master    #1827      +/-   ##
==========================================
+ Coverage   70.28%   70.31%   +0.03%     
==========================================
  Files         148      148              
  Lines       18621    18648      +27     
==========================================
+ Hits        13087    13113      +26     
- Misses       5534     5535       +1

@astafan8
Copy link
Contributor

@Dominik-Vogel I merged the master with the _Cache into this branch (didn't want to bother with rebases), and i think it looks good. Let's have another last look and merge (if CI allows :) )

@astafan8 astafan8 changed the title Cache for DelegateParameter Implement cache for DelegateParameter to mirror cache of the source parameter Nov 21, 2019
Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

I think this is good to go. @Dominik-Vogel please also confirm.

@Dominik-Vogel Dominik-Vogel merged commit 2714200 into microsoft:master Nov 22, 2019
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.

Bug in DelegateParameter: get_latest and set_cache are not forwarded to source parameter
2 participants