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.

Sign up for GitHub

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

Jump to bottom

Dont read from layouts and dependencies tables in QCoDeS #1572

Merged
merged 19 commits into from
Jun 7, 2019
Merged

Dont read from layouts and dependencies tables in QCoDeS #1572

merged 19 commits into from
Jun 7, 2019

Conversation

astafan8
Copy link
Contributor

@astafan8 astafan8 commented May 20, 2019 •
edited

This PR removes usage of functions which read information from layouts and dependencies tables. Reading information from layouts and dependencies tables is substituted with reading RunDescriber object (stored in run_description column of runs table). This PR does NOT remove writing to layouts and dependencies tables, this will be done in another PR.

The big news in this PR is that get_data_by_id and thus plot_by_id is now free from using the layouts and dependencies tables.

This PR is just one of the PRs on the way towards removing the layouts and dependencies tables completely from QCoDeS database.

ToDo:

  • whenever necessary remove usage of private attributes to access RunDescriber and/or InterDependencies
  • flatten data arrays get_data_by_id or not? the tests of get_data_by_id say "yes". but perhaps when plot_by_id is improved (that's in a different PR) then it can use get_parameter_data directly, and then get_data_by_id can be deprecated; but within the scope of this PR we just make sure that the tests work and plotting notebooks still produce correct plots (since plot_by_id uses get_data_by_id that is drastically adjusted in this PR)

@astafan8 astafan8 force-pushed the dont-use-layout-dependencies branch from 6ea5f02 to 7b76730 Compare May 20, 2019 13:12
@astafan8
Add non_dependencies property to InterDependecies_
0b6f598 
to substitute sqlite->get_non_dependencies
@astafan8
Use new API (InterDependencies_) in sqlite get_parameter_data
82dcee2 
... instead of get_non_dependencies, get_parameter_dependencies
@astafan8
Use InterDependencies_ in DataSet.get_parameters, no sqlite_base stuff
fd5cb8a
@astafan8
Use InterDependencies_ in DataSet.get_parameter_data
c640e9b 
instead of sqlite get_non_dependencies
@astafan8
Use InterDependencies_ in test_get_parameter_data_independent_param..
241b9e4 
instead of sqlite get_non_dependencies
@astafan8
Use InterDependencies_ in sqlite_base test get_parameter_data...
5a8c441 
instead of sqlite get_non_dependencies
@astafan8
Rewrite get_data_by_id to use get_parameter_data, InterDependencies_
945ec05 
instead of sqlite get_dependents, get_dependencies, get_layout (and
also DataSet.get_values, DataSet.get_setpoints, and
flatten_1D_data_for_plot).
@astafan8
Raise DeprecationWarning in layouts/dependencies-tables-related funcs
d06b868 
The following functions have raising DeprecationWarning at the
beginning of the their bodies:
- get_layout
- get_non_dependencies
- get_parameter_dependencies

The following functions were made 'private' (using underscore in
their name) and the 'public' versions of those functions were made to
raise the DeprecationWarning (this is done in order to detect if
qcodes still has code that uses these functions which read from
layouts and dependencies tables):
- get_layout_id
- get_dependents
- get_dependencies
- get_parameters
- get_paramspec
@astafan8
Test InterDependencies_.non_dependencies property
91b4695
@astafan8 astafan8 force-pushed the dont-use-layout-dependencies branch from 7b76730 to 91b4695 Compare May 27, 2019 14:28
@astafan8 astafan8 changed the title Dont read from layouts and dependencies tables in qcodes Dont read from layouts and dependencies tables in QCoDeS May 29, 2019
@WilliamHPNielsen WilliamHPNielsen marked this pull request as ready for review June 7, 2019 09:47
@WilliamHPNielsen
Copy link
Contributor

WilliamHPNielsen commented Jun 7, 2019 •
edited

I'd say this PR is ready for review. Note that it is not completely true that we don't read from layouts and dependencies; there is a single place (in the current API) where that happens: in the create_run function from sqlite.queries.

I think the big news in this PR is that get_data_by_id and thus plot_by_id is now free from using the layouts and dependencies tables.

@QCoDeS/core

@astafan8
Copy link
Contributor Author

astafan8 commented Jun 7, 2019

there is a single place (in the current API) where we still read from layouts and dependencies: in the create_run function from sqlite.queries

do you mean the _get_layout_id inside _add_parameters_to_layout_and_deps inside _insert_run? if so, that's fine, but this "reading" is now done only with "private" methods of sqlite.queries and it will go away together with the tables (because everything will be tied to run_description column)

astafan8
astafan8 commented Jun 7, 2019
View reviewed changes
Copy link
Contributor Author

@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.

as the original creator of this PR i can't approve it, so i just approve it in a comment here :)

WilliamHPNielsen
WilliamHPNielsen approved these changes Jun 7, 2019
View reviewed changes
Copy link
Contributor

@WilliamHPNielsen WilliamHPNielsen left a comment

Choose a reason for hiding this comment

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

LGTM

@WilliamHPNielsen WilliamHPNielsen merged commit a4697d4 into microsoft:master Jun 7, 2019
giulioungaretti pushed a commit that referenced this pull request Jun 7, 2019
Generated gh-pages for commit a4697d4
f9a502e 
Merge: 16655ea 7b26ba1
Author: William H.P. Nielsen <whpn@mailbox.org>

    Merge pull request #1572 from astafan8/dont-use-layout-dependencies
@astafan8 astafan8 deleted the dont-use-layout-dependencies branch July 22, 2019 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WilliamHPNielsen WilliamHPNielsen WilliamHPNielsen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@astafan8 @WilliamHPNielsen