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

Fix/refactor videomode folder #237

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

Conversation

TheoLaudatQM
Copy link
Contributor

Rename the module called video_mode into live_mode to avoid confusion with the video mode tool used to tune up quantum dots devices.
We can still debate on the name if you think that another one would be more relevant for this tool which is used for updating parameters from a QUA program while the program is running in an asynchronous manner.

  • Deprecation warnings when trying to import qualang_tools.video_mode

Copy link

github-actions bot commented Oct 23, 2024

Unit Test Results

412 tests   403 ✔️  48s ⏱️
    1 suites      9 💤
    1 files        0

Results for commit fe97a66.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@nulinspiratie nulinspiratie left a comment

Choose a reason for hiding this comment

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

What about if we have the video_mode/__init__.py import the original functions/classes which are now in live_mode? This way the code remains backwards compatible

@TheoLaudatQM
Copy link
Contributor Author

@nulinspiratie What I had in mind was this

import warnings
warnings.warn("the video_mode module is deprecated, please import live_mode instead", DeprecationWarning,
              stacklevel=0)
from qualang_tools.live_mode.livemode import LiveMode, ParameterTable

__all__ = ["LiveMode", "ParameterTable"]

But

  1. the warning doesn't show up when calling from qualang_tools.video_mode import *
  2. The user will still get error when running his previous scripts that most likely use from qualang_tools.video_mode import VideoMode, ParameterTable

@nulinspiratie
Copy link
Contributor

@TheoLaudatQM looks like a nice solution! All good from my end

@nulinspiratie nulinspiratie self-requested a review October 25, 2024 09:45
@yomach
Copy link
Collaborator

yomach commented Oct 27, 2024

@nulinspiratie What I had in mind was this

import warnings
warnings.warn("the video_mode module is deprecated, please import live_mode instead", DeprecationWarning,
              stacklevel=0)
from qualang_tools.live_mode.livemode import LiveMode, ParameterTable

__all__ = ["LiveMode", "ParameterTable"]

But

1. the warning doesn't show up when calling `from qualang_tools.video_mode import *`

2. The user will still get error when running his previous scripts that most likely use `from qualang_tools.video_mode import VideoMode, ParameterTable`
  1. I am 99% it is possible to display a warning in this case, I can take a look if you want.
  2. This can be solved by doing: from qualang_tools.live_mode.livemode import LiveMode as VideoMode.

We can merge if we're okay with the breaking changes and the lack of backward compatibility. Otherwise, we can make it compatible.

@nulinspiratie
Copy link
Contributor

Hmm, if we want to maintain backwards compatibility, we could also opt for the following:

video_mode/videomode.py

from qualang_tools.live_mode import LiveMode
import warnings


class VideoMode(LiveMode):
    def __init__(self, qm: QuantumMachine, parameters: Union[Dict, ParameterTable]):
        warnings.warn("the video_mode module is deprecated, please import live_mode instead", DeprecationWarning,
              stacklevel=0)
        super().__init__(self, qm, parameters)

We would then need to rename the new VideoMode to something else, maybe VideoMode2D?

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.

3 participants