Skip to content

Replace qq auto with qq run - #993

Merged
Edoardo-Pedicillo merged 4 commits into
mainfrom
qq_run
Nov 4, 2024
Merged

Replace qq auto with qq run#993
Edoardo-Pedicillo merged 4 commits into
mainfrom
qq_run

Conversation

@Edoardo-Pedicillo

@Edoardo-Pedicillo Edoardo-Pedicillo commented Oct 2, 2024

Copy link
Copy Markdown
Contributor

This choice comes from some internal discussions, the most recent one is qiboteam/xmind-diagrams#31 (comment)

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.
  • Compatibility with Qibo modules (Please edit this section if the current pull request is not compatible with the following branches).
    • Qibo: master
    • Qibolab: 0.1
    • Qibolab_platforms_qrc: main

@codecov

codecov Bot commented Oct 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.32%. Comparing base (39e5d3c) to head (1e6d06c).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/qibocal/cli/_base.py 91.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #993      +/-   ##
==========================================
- Coverage   97.33%   97.32%   -0.01%     
==========================================
  Files         120      120              
  Lines        9747     9756       +9     
==========================================
+ Hits         9487     9495       +8     
- Misses        260      261       +1     
Flag Coverage Δ
unittests 97.32% <92.30%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/qibocal/cli/run.py 96.15% <100.00%> (ø)
src/qibocal/cli/_base.py 92.40% <91.66%> (-0.46%) ⬇️

@Edoardo-Pedicillo

Copy link
Copy Markdown
Contributor Author

@scarrazza do you think we can go on with this PR and update the Qibocal-paper before the submission?

@scarrazza

Copy link
Copy Markdown
Member

Yes, I agree.

@Edoardo-Pedicillo
Edoardo-Pedicillo marked this pull request as ready for review October 21, 2024 13:21

@andrea-pasquale andrea-pasquale left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @Edoardo-Pedicillo.
In theory we need to update the diagram that we have in the qibocal paper also here in the documenation, correct? Maybe it is worth waiting until the diagram is ready to merge the PR.

Also, perhaps we could add a log message when importing the library saying that qq auto has been renamed qq run? It could also makes sense to do it once we have a release I guess. I'm suggesting this to lower the confusion among users :)

@alecandido

Copy link
Copy Markdown
Member

Also, perhaps we could add a log message when importing the library saying that qq auto has been renamed qq run? It could also makes sense to do it once we have a release I guess. I'm suggesting this to lower the confusion among users :)

I would exclude doing that at import time (it would appear also in any script not using the CLI), and I'm generally against warnings.

What I may support is to keep the qq auto subcommand, just to print a message telling the user to use qq run instead.

@andrea-pasquale

Copy link
Copy Markdown
Contributor

What I may support is to keep the qq auto subcommand, just to print a message telling the user to use qq run instead.

This could work.

@Edoardo-Pedicillo Edoardo-Pedicillo added this to the Qibocal 0.1.1 milestone Oct 28, 2024
@Edoardo-Pedicillo
Edoardo-Pedicillo added this pull request to the merge queue Nov 4, 2024
Merged via the queue into main with commit 2cb2090 Nov 4, 2024
@Edoardo-Pedicillo
Edoardo-Pedicillo deleted the qq_run branch November 4, 2024 13:54
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.

4 participants