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

Attach default experiment to a measurement #4956

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

einsmein
Copy link

This PR finds a default experiment and attach it to a measurement when it is created.
This behavior matches the document of Measurement class , which states

...

    Args:
        exp: Specify the experiment to use. If not given
            the default one is used. The default experiment
            is the latest one created.
...

@jenshnielsen
Copy link
Collaborator

@einsmein QCoDeS is using darker https://pypi.org/project/darker/ to only format changes in order to not obscure the history so please don't format the entire file when making small changes

@jenshnielsen
Copy link
Collaborator

    Args:
        exp: Specify the experiment to use. If not given
            the default one is used. The default experiment
            is the latest one created.

This logic is already implemented but it lives in the dataset class. See line 272 in https://github.com/QCoDeS/Qcodes/blob/master/qcodes/dataset/data_set.py

@einsmein
Copy link
Author

@jenshnielsen I understand a default experiment is attached to a dataset. However, measurement.experiment is empty even though the underlying dataset is attached to a default experiment, which can be misleading at runtime imo.

@jenshnielsen
Copy link
Collaborator

@jenshnielsen I understand a default experiment is attached to a dataset. However, measurement.experiment is empty even though the underlying dataset is attached to a default experiment, which can be misleading at runtime imo.

I agree, experiment really should have been a private property on the measurement class. There is no reason why the users should need to interact with it there. But since its not I am happy with this change provided:

  • It gets a test
  • The other tests are fixed to pass
  • The unrelated formatting is removed

@einsmein
Copy link
Author

einsmein commented Jan 30, 2023

I'm working on fixing all those to fix CI. Now that you mentioned that it's meant to be private, I may down-prioritize it. What is experiment attribute for? Is it only to propagate it down to dataset?

@astafan8 I think it is expected that an experiment, to which a measurement (and thus, dataset it produces) is attached, should be decided at initialization of the measurement object. Having one measurement producing dataset to different experiments depending on when dataset is being saved would be confusing. What's your thoughts on this?

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.

2 participants