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

Version RunDescriber, and 5->6 DB upgrade, also reorg run descriptions module structure #1577

Merged

Conversation

WilliamHPNielsen
Copy link
Contributor

@WilliamHPNielsen WilliamHPNielsen commented May 23, 2019

This is a proposal for how the RunDescriber object could be versioned. Hopefully, this will allow us to decouple changes to the RunDescriber object from the SQLite schema version.

Changes proposed in this pull request:

  • new descriptions package where all the RunDescriber/InterDependencies/ParamSpec/etc go
  • Introduce a separate serialization layer for RunDescriber
  • Introduce a "bone yard" of old versions of RunDescriber in versioning package
  • RunDescriber with InterDependencies (no underscore) becomes version 0, and gets the version property as well
  • RunDescriber with InterDependencies_ (with underscore) becomes version 1 (obviously the version property as well)
  • A DB upgrade 5 -> 6 is introduced that takes a RunDesscription from the database, and makes it RunDescriber version 0, which is just adding the 'version' with the value of 0
  • also includes a refactoring of some test fixtures into a separate file
  • cleanup of imports of many example notebooks (i.e. qcodes.dataset.sqlite_base, qcodes.dataset.database, qcodes.dataset.param_spec)
  • rename serialize to _to_dict and deserialize to _from_dict for RunDescriber (all versions), InterDependencies_, InterDependencies, ParamSpecBase (also make those methods private)
  • remove usage of imports from qcodes.dataset.sqlite_base, qcodes.dataset.database, qcodes.dataset.param_spec in notebooks and in code

This PR is originally based on #1566.

@WilliamHPNielsen WilliamHPNielsen force-pushed the dev/reorg_runs_descriptions branch 2 times, most recently from 04571f1 to accb981 Compare May 24, 2019 09:57
@WilliamHPNielsen WilliamHPNielsen marked this pull request as ready for review May 24, 2019 10:14
@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

Merging #1577 into master will increase coverage by 0.14%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master    #1577      +/-   ##
==========================================
+ Coverage   72.18%   72.33%   +0.14%     
==========================================
  Files         112      116       +4     
  Lines       12293    12388      +95     
==========================================
+ Hits         8874     8961      +87     
- Misses       3419     3427       +8

@astafan8 astafan8 force-pushed the dev/reorg_runs_descriptions branch from 5b14fe9 to 61d0bec Compare May 27, 2019 13:24
... and make update_run_description use _update_run_description
internally.

If _update_run_description becomes useful in too many place it can be
promoted to be non-private and perhaps get a better name.
as introduced by one of the previous commits. writing the version
field is probably not harmful, but inconsistent with the original
behavior of the function, hence this commits removes this
inconsistency.
To ensure that the fix does indeed what it's supposed to.
The logic of conversion between the wrongly written run_description
to what it should be is a bit involved since we are using conversion
functions from serializion.py module. So we factor out that code into
its own function and document it heavily.
@astafan8 astafan8 force-pushed the dev/reorg_runs_descriptions branch from 03a35e5 to cc8465b Compare June 3, 2019 16:54
See the updated docstring of the module for the info on the
convention.
@astafan8 astafan8 force-pushed the dev/reorg_runs_descriptions branch from 18b0239 to acfb5d5 Compare June 3, 2019 17:33
@WilliamHPNielsen
Copy link
Contributor Author

This looks good! Let's get it in.

@WilliamHPNielsen WilliamHPNielsen merged commit 783abd6 into microsoft:master Jun 4, 2019
giulioungaretti pushed a commit that referenced this pull request Jun 4, 2019
Merge: a4d027b acfb5d5
Author: William H.P. Nielsen <whpn@mailbox.org>

    Merge pull request #1577 from WilliamHPNielsen/dev/reorg_runs_descriptions
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