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

Add _Cache to _BaseParameter, use it in GetLatest, remove set_cache method, remove _latest #1832

Merged
merged 24 commits into from
Nov 21, 2019

Conversation

astafan8
Copy link
Contributor

@astafan8 astafan8 commented Nov 20, 2019

Main thing

This PR takes another approach to #1791 - introducing _Cache class which:

This PR subsumes significant part of #1828 but not the improvements to snapshot_base.

Some details of this PR

  • GetLatest and get_latest remain as is for backwards compatibility, but under the hood they reuse _Cache and its methods "as-is". In the near future we will be able to deprecate GetLatest altogether
    and move our users to use .cache only.
  • Note that _Cache.get as opposed to get_latest.get has an additional get_if_invalid flag that is useful to set to False when you don't want to call get of the parameter if the cached value is invalid (for example, because the parameter has never been captured).
  • All docstrings in parameter.py were rewritten to reflect the existence of the _Cache
  • Drivers which used set_cache were changed to use cache.set.
  • Closes _BaseParameter: add get_cache* API and use it in GetLatest and in snapshot_base #1791 because it subsumes it.
  • refactoring test_parameter.py into a package is out of scope for this PR

To be done in another PR:

  • rewrite test_parameter.py to test cache.* instead of get_latest.* but also add minor tests for get_latest to ensure that it does the same as cache

@codecov
Copy link

codecov bot commented Nov 20, 2019

Codecov Report

Merging #1832 into master will increase coverage by 0.03%.
The diff coverage is 88.75%.

@@            Coverage Diff             @@
##           master    #1832      +/-   ##
==========================================
+ Coverage   70.24%   70.28%   +0.03%     
==========================================
  Files         148      148              
  Lines       18598    18621      +23     
==========================================
+ Hits        13064    13087      +23     
  Misses       5534     5534

Copy link
Contributor

@Dominik-Vogel Dominik-Vogel left a comment

Choose a reason for hiding this comment

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

Great! and good job with removing _latest from all the tests!

qcodes/instrument/parameter.py Show resolved Hide resolved
qcodes/instrument/parameter.py Outdated Show resolved Hide resolved
qcodes/instrument/parameter.py Show resolved Hide resolved
qcodes/instrument/parameter.py Outdated Show resolved Hide resolved
@astafan8 astafan8 merged commit e3c5182 into microsoft:master Nov 21, 2019
@astafan8 astafan8 deleted the cache-of-parameter branch November 21, 2019 16:02
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.

None yet

3 participants