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

Feature / database extract runs #1397

Merged

Conversation

WilliamHPNielsen
Copy link
Contributor

@WilliamHPNielsen WilliamHPNielsen commented Nov 27, 2018

Fixes #1217

Changes proposed in this pull request:

  • Add a function, extract_runs_into_db that takes a set of run_ids from one database file and inserts those runs into another database file.
  • Add a notebook explaining this function

The current limitations and behaviours are:

  • All runs to be inserted (in one function call) must come from the same experiment
  • The source database must be of the newest version. If it is not, calling the extraction function is a NOOP, and the user will be warned that an upgrade is needed. A boolean kwarg allows the user to have extract_runs_into_db upgrade the source DB.
  • The target DB will be created if it does not exist. If it does exist, it will be upgraded and is in an old version, the user will be warned similarly to how it works for the source DB.

@QCoDeS/core

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.

This is ready to go! It was a great freaking piece of work. I tested successfully on actual real .db files, and now everything looks fine :) Time to celebrate!

@WilliamHPNielsen
Copy link
Contributor Author

@QCoDeS/core shall we merge once CI (minus codacy, who doesn't like fixtures) is happy?

@WilliamHPNielsen WilliamHPNielsen merged commit d20647c into microsoft:master Nov 30, 2018
@WilliamHPNielsen WilliamHPNielsen deleted the feat/database_copy_paste_runs branch November 30, 2018 12:50
giulioungaretti pushed a commit that referenced this pull request Nov 30, 2018
Merge: b0ad951 557e627
Author: William H.P. Nielsen <whpn@mailbox.org>

    Merge pull request #1397 from WilliamHPNielsen/feat/database_copy_paste_runs
@ThorvaldLarsen
Copy link
Contributor

@WilliamHPNielsen Concerning this comment when extracting to different databases:

"Note that the runs will have different run_ids in the new database. Their GUIDs are, however, the same (as they must be)."

This can cause a lot of confusion as currently the run_id and sample name are the unique identifiers that we use for a measurement run (that is the reason to have this information printed in the title of all plots). Is the idea that we should be using the guid as the unique identifier when putting datasets into presentations?
I think we should make sure that either the run_id sample name stays unique across all databases or change the identifier we use.

I am happy to meet and discuss/learn if these concerns have already been addressed elsewhere.

@WilliamHPNielsen
Copy link
Contributor Author

@ThorvaldLarsen as discussed offline yesterday, the core team will get together and attack this. But just for completeness, I note here that I currently think changing the identifier you use is better. The dataset name seems like a good pick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants