Skip to content

Remove execution result from notebooks - #5326

Merged
CirqBot merged 11 commits into
quantumlib:masterfrom
dabacon:executeresult
Jun 7, 2022
Merged

CirqBot merged 11 commits into
quantumlib:masterfrom
dabacon:executeresult

Conversation

@dabacon

@dabacon dabacon commented May 3, 2022

Copy link
Copy Markdown
Collaborator

This removes execution results from notebooks.

@MichaelBroughton these are causing downstream failure with docs build. Where do I figure out if these are being built when we push these out? i.e. I'm not certain that the azure notebooks get built with results and don't want to strip these results if this actually isn't built.

@dabacon
dabacon requested a review from viathor May 3, 2022 22:28
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CirqBot CirqBot added the size: L 250< lines changed <1000 label May 3, 2022
@dabacon
dabacon requested a review from MichaelBroughton May 3, 2022 22:29
@dstrain115

Copy link
Copy Markdown
Collaborator

Are we sure that this example will run in devsite? I seem to recall that we saved outputs on the azure notebook to avoid trying to execute it during CI

@dabacon

dabacon commented May 3, 2022

Copy link
Copy Markdown
Collaborator Author

Are we sure that this example will run in devsite? I seem to recall that we saved outputs on the azure notebook to avoid trying to execute it during CI

That was the question I asked @MichaelBroughton . Where do we find excluded notebooks?

@dabacon

dabacon commented May 3, 2022

Copy link
Copy Markdown
Collaborator Author

I take it from your comment @dstrain115 that if it has executed code it doesn't execute it during the CI? Seems very fragile to me.

I'll remove Azure and IonQ as I think those are likely to need the executed code.

@dstrain115

Copy link
Copy Markdown
Collaborator

Yes, if we store executed results, they are not executed in dev site.

@dabacon

dabacon commented May 11, 2022

Copy link
Copy Markdown
Collaborator Author

I've removed the notebooks that talk to services.

@dabacon dabacon added size: M 50< lines changed <250 and removed size: L 250< lines changed <1000 labels May 12, 2022
@dabacon

dabacon commented May 12, 2022

Copy link
Copy Markdown
Collaborator Author

CirqBot you are bad at judging size.

@MichaelBroughton MichaelBroughton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These changes look good. Can we just outright cut the ion_device tutorial. Things are looking pretty outdated there. Thoughts @dstrain115 ?

@dabacon dabacon added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 7, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 7, 2022
@dabacon

dabacon commented Jun 7, 2022

Copy link
Copy Markdown
Collaborator Author

Filing #5463 to decide about ion device notebook

@CirqBot
CirqBot merged commit d8f6043 into quantumlib:master Jun 7, 2022
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jun 7, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
This removes execution results from notebooks.

@MichaelBroughton these are causing downstream failure with docs build.  Where do I figure out if these are being built when we push these out?  i.e. I'm not certain that the azure notebooks get built with results and don't want to strip these results if this actually isn't built.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M 50< lines changed <250

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants