Skip to content

Comments

feature: Add png for jupyter nb and saving#382

Merged
giulioungaretti merged 5 commits intomasterfrom
feature/save_QT
Nov 9, 2016
Merged

feature: Add png for jupyter nb and saving#382
giulioungaretti merged 5 commits intomasterfrom
feature/save_QT

Conversation

@giulioungaretti
Copy link
Contributor

@giulioungaretti giulioungaretti commented Nov 7, 2016 •
edited
Loading

Fixes #360 and hints at an easy solution for #376 .

Changes proposed in this pull request:

  • add _repr_png
  • add save method
  • add auto save feature to loop by expanding with_bg_task

Refactoring QT in an idoimatic way will take a bit more time, this at least lessens the pain for say @nataliejpg and some other experimentalist.

@giulioungaretti giulioungaretti self-assigned this Nov 7, 2016
giulioungaretti
giulioungaretti commented Nov 7, 2016
View reviewed changes
qcodes/plots/pyqtgraph.py
buffer.open(self.rpg.QtCore.QIODevice.ReadWrite)
image.save(buffer, 'PNG')
buffer.close()
return bytes(byte_array._getValue())
Copy link
Contributor Author

@giulioungaretti giulioungaretti Nov 7, 2016

Choose a reason for hiding this comment

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

note to self: this is a gnarly hack IMHO, but then again no other way around.

@giulioungaretti
feature: Add save to all plotters
eba6e95
@giulioungaretti
feature: Add end_task
55eab24 
Allow to programmatically do something after the loop is done.
F.ex save the plot.
@giulioungaretti
docs: Add save example, use faster refresh rate
dff292b 
The default was 1 second, but I think aiming for 100hz, and using
that as default is a bit closer to reality.

Then in the examples we can just use the default which makes for
less typing for the users!
@giulioungaretti
Copy link
Contributor Author

giulioungaretti commented Nov 8, 2016

@alan-geller can you look at 55eab24 and dff292b , since it touches your code :D

It just adds the option to run a task at the end of the loop or active loop , which is really useful to automatically save plots and sets the default refresh rate to around 100hz (which is kind of a basic golden standard, IMHO) !

alan-geller
alan-geller suggested changes Nov 8, 2016
View reviewed changes
Copy link

@alan-geller alan-geller left a comment

Choose a reason for hiding this comment

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

Looks fine, but I'd make some doc string changes to make it a bit clearer for the user.

qcodes/loops.py Outdated
bg_task=self.bg_task, end_task=self.end_task, bg_min_delay=self.bg_min_delay)

def with_bg_task(self, task, min_delay=1):
def with_bg_task(self, task, end_task=None, min_delay=1):
Copy link

@alan-geller alan-geller Nov 8, 2016

Choose a reason for hiding this comment

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

@giulioungaretti I think this works fine, but I'm not sure the two features (background task and end task) should be joined at the hip like this. If we think of this end_task as specifically finishing whatever was done by the background task, which seems reasonable to me, then linking them is fine, but perhaps a more descriptive name for end_task would help make it clearer to the user. bg_final_task? bg_task_finish? end_bg_task?

Copy link
Contributor Author

@giulioungaretti giulioungaretti Nov 8, 2016

Choose a reason for hiding this comment

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

@alan-geller agree, and yes I think that's how one should think and then yes, bg_final_task is just perfect!

qcodes/loops.py
invoked at the end of the measurement loop.

min_delay (default 1): The minimum number of seconds to wait
min_delay (default 0.01): The minimum number of seconds to wait
Copy link

@alan-geller alan-geller Nov 8, 2016

Choose a reason for hiding this comment

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

I had originally set this to 1 because anything shorter caused the loop to slow appreciably when using matplot. With qtplot it's not a problem. I'm fine with being more aggressive with the default, but I think in the doc string perhaps we should suggest that users lengthen the min_delay if the background task is doing a lot of processing, such as updating a matplot graph.

Copy link
Contributor Author

@giulioungaretti giulioungaretti Nov 8, 2016

Choose a reason for hiding this comment

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

That is indeed a problem with matplotlib, yes we should definitely add that to avoid nasty surprises!

qcodes/loops.py Outdated
invoked periodically during the measurement loop.

min_delay (default 1): The minimum number of seconds to wait
end_task: A callable object with no parameters. This object will be
Copy link

@alan-geller alan-geller Nov 8, 2016

Choose a reason for hiding this comment

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

I think the doc string here should clarify that end_task is intended to clean up after or otherwise finish the background task work, rather than being for general cleanup.

Copy link
Contributor Author

@giulioungaretti giulioungaretti Nov 8, 2016

Choose a reason for hiding this comment

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

👍

alan-geller
alan-geller approved these changes Nov 9, 2016
View reviewed changes
Copy link

@alan-geller alan-geller left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@giulioungaretti giulioungaretti merged commit 7fb4e5c into master Nov 9, 2016
@giulioungaretti giulioungaretti deleted the feature/save_QT branch December 8, 2016 14:04
bors bot added a commit that referenced this pull request Sep 6, 2022
@bors @dependabot @jenshnielsen
Merge #4559
ec858a5 
4559: Update towncrier requirement from ~=21.9.0 to ~=22.8.0 r=jenshnielsen a=dependabot[bot]

Updates the requirements on [towncrier](https://github.com/hawkowl/towncrier) to permit the latest version.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/hawkowl/towncrier/releases">towncrier's releases</a>.</em></p>
<blockquote>
<h2>Towncrier 22.8.0</h2>
<h1>towncrier 22.8.0 (2022-08-29)</h1>
<p>No significant changes since the previous release candidate.</p>
<h1>towncrier 22.8.0.rc1 (2022-08-28)</h1>
<h2>Features</h2>
<ul>
<li>
<p>Make the check subcommand succeed for branches that change the news file</p>
<p>This should enable the <code>check</code> subcommand to be used as a CI lint step and
not fail when a pull request only modifies the configured news file (i.e. when
the news file is being assembled for the next release). (<code>[#337](twisted/towncrier#337) &lt;https://github.com/hawkowl/towncrier/issues/337&gt;</code>_)</p>
</li>
<li>
<p>Added support to tables in toml settings, which provides a more intuitive
way to configure custom types. (<code>[#369](twisted/towncrier#369) &lt;https://github.com/hawkowl/towncrier/issues/369&gt;</code>_)</p>
</li>
<li>
<p>The <code>towncrier create</code> command line now has a new <code>-m TEXT</code> argument that is used to define the content of the newly created fragment. (<code>[#374](twisted/towncrier#374) &lt;https://github.com/hawkowl/towncrier/issues/374&gt;</code>_)</p>
</li>
</ul>
<h2>Bugfixes</h2>
<ul>
<li>
<p>The extra newline between the title and rendered content when using <code>--draft</code> is no longer inserted. (<code>[#105](twisted/towncrier#105) &lt;https://github.com/hawkowl/towncrier/issues/105&gt;</code>_)</p>
</li>
<li>
<p>The detection of duplicate release notes was fixed and recording changes of same version is no longer triggered.</p>
<p>Support for having the release notes for each version in a separate file is working again. This is a regression introduced in VERSION 19.9.0rc1. (<code>[#391](twisted/towncrier#391) &lt;https://github.com/hawkowl/towncrier/issues/391&gt;</code>_)</p>
</li>
</ul>
<h2>Improved Documentation</h2>
<ul>
<li>Improve <code>CONTRIBUTING.rst</code> and add PR template. (<code>[#342](twisted/towncrier#342) &lt;https://github.com/hawkowl/towncrier/issues/342&gt;</code>_)</li>
<li>Move docs too the main branch and document custom fragment types. (<code>[#367](twisted/towncrier#367) &lt;https://github.com/hawkowl/towncrier/issues/367&gt;</code>_)</li>
<li>The CLI help messages were updated to contain more information. (<code>[#384](twisted/towncrier#384) &lt;https://github.com/hawkowl/towncrier/issues/384&gt;</code>_)</li>
</ul>
<h2>Deprecations and Removals</h2>
<ul>
<li>Support for all Python versions older than 3.7 has been dropped. (<code>[#378](twisted/towncrier#378) &lt;https://github.com/hawkowl/towncrier/issues/378&gt;</code>_)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/twisted/towncrier/blob/trunk/NEWS.rst">towncrier's changelog</a>.</em></p>
<blockquote>
<h1>towncrier 22.8.0 (2022-08-29)</h1>
<p>No significant changes since the previous release candidate.</p>
<h1>towncrier 22.8.0.rc1 (2022-08-28)</h1>
<h2>Features</h2>
<ul>
<li>
<p>Make the check subcommand succeed for branches that change the news file</p>
<p>This should enable the <code>check</code> subcommand to be used as a CI lint step and
not fail when a pull request only modifies the configured news file (i.e. when
the news file is being assembled for the next release). (<code>[#337](twisted/towncrier#337) &lt;https://github.com/hawkowl/towncrier/issues/337&gt;</code>_)</p>
</li>
<li>
<p>Added support to tables in toml settings, which provides a more intuitive
way to configure custom types. (<code>[#369](twisted/towncrier#369) &lt;https://github.com/hawkowl/towncrier/issues/369&gt;</code>_)</p>
</li>
<li>
<p>The <code>towncrier create</code> command line now has a new <code>-m TEXT</code> argument that is used to define the content of the newly created fragment. (<code>[#374](twisted/towncrier#374) &lt;https://github.com/hawkowl/towncrier/issues/374&gt;</code>_)</p>
</li>
</ul>
<h2>Bugfixes</h2>
<ul>
<li>
<p>The extra newline between the title and rendered content when using <code>--draft</code> is no longer inserted. (<code>[#105](twisted/towncrier#105) &lt;https://github.com/hawkowl/towncrier/issues/105&gt;</code>_)</p>
</li>
<li>
<p>The detection of duplicate release notes was fixed and recording changes of same version is no longer triggered.</p>
<p>Support for having the release notes for each version in a separate file is working again. This is a regression introduced in VERSION 19.9.0rc1. (<code>[#391](twisted/towncrier#391) &lt;https://github.com/hawkowl/towncrier/issues/391&gt;</code>_)</p>
</li>
</ul>
<h2>Improved Documentation</h2>
<ul>
<li>Improve <code>CONTRIBUTING.rst</code> and add PR template. (<code>[#342](twisted/towncrier#342) &lt;https://github.com/hawkowl/towncrier/issues/342&gt;</code>_)</li>
<li>Move docs too the main branch and document custom fragment types. (<code>[#367](twisted/towncrier#367) &lt;https://github.com/hawkowl/towncrier/issues/367&gt;</code>_)</li>
<li>The CLI help messages were updated to contain more information. (<code>[#384](twisted/towncrier#384) &lt;https://github.com/hawkowl/towncrier/issues/384&gt;</code>_)</li>
</ul>
<h2>Deprecations and Removals</h2>
<ul>
<li>Support for all Python versions older than 3.7 has been dropped. (<code>[#378](twisted/towncrier#378) &lt;https://github.com/hawkowl/towncrier/issues/378&gt;</code>_)</li>
</ul>
<h2>Misc</h2>
<ul>
<li><code>[#292](twisted/towncrier#292) &lt;https://github.com/hawkowl/towncrier/issues/292&gt;</code><em>, <code>[#330](twisted/towncrier#330) &lt;https://github.com/hawkowl/towncrier/issues/330&gt;</code></em>, <code>[#366](twisted/towncrier#366) &lt;https://github.com/hawkowl/towncrier/issues/366&gt;</code><em>, <code>[#376](twisted/towncrier#376) &lt;https://github.com/hawkowl/towncrier/issues/376&gt;</code></em>, <code>[#377](twisted/towncrier#377) &lt;https://github.com/hawkowl/towncrier/issues/377&gt;</code><em>, <code>[#380](twisted/towncrier#380) &lt;https://github.com/hawkowl/towncrier/issues/380&gt;</code></em>, <code>[#381](twisted/towncrier#381) &lt;https://github.com/hawkowl/towncrier/issues/381&gt;</code><em>, <code>[#382](twisted/towncrier#382) &lt;https://github.com/hawkowl/towncrier/issues/382&gt;</code></em>, <code>[#383](twisted/towncrier#383) &lt;https://github.com/hawkowl/towncrier/issues/383&gt;</code><em>, <code>[#393](twisted/towncrier#393) &lt;https://github.com/hawkowl/towncrier/issues/393&gt;</code></em>, <code>[#399](twisted/towncrier#399) &lt;https://github.com/hawkowl/towncrier/issues/399&gt;</code><em>, <code>[#402](twisted/towncrier#402) &lt;https://github.com/hawkowl/towncrier/issues/402&gt;</code></em></li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/twisted/towncrier/commit/411f2676b18daf9ba1eb656f7dc6c726397c4a9e"><code>411f267</code></a> Update version for final release.</li>
<li><a href="https://github.com/twisted/towncrier/commit/f031dfa94e52e07b6c71e88cd5cd50e3803ad96c"><code>f031dfa</code></a> towncrier build --yes</li>
<li><a href="https://github.com/twisted/towncrier/commit/4c854f58a2074458cd66767aca9f3aaa34d842d8"><code>4c854f5</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/hawkowl/towncrier/issues/402">#402</a> from webknjaz/patch-2</li>
<li><a href="https://github.com/twisted/towncrier/commit/f911402b69960bee358b0c2011d685721afa9454"><code>f911402</code></a> Pin external github actions,</li>
<li><a href="https://github.com/twisted/towncrier/commit/a814202234214182855fe4a5ffb40c7f7aec2b04"><code>a814202</code></a> [pre-commit.ci] auto fixes from pre-commit.com hooks</li>
<li><a href="https://github.com/twisted/towncrier/commit/675219a6deac7c65bb94b72da55059b7077e5ec4"><code>675219a</code></a> Add a dummy change note for PR <a href="https://github-redirect.dependabot.com/hawkowl/towncrier/issues/402">#402</a></li>
<li><a href="https://github.com/twisted/towncrier/commit/b566bfb0dcdcd86fa9d349875eba15e647c06459"><code>b566bfb</code></a> Uninvent the wheel w/ <code>re-actors/alls-green</code> @ GHA</li>
<li><a href="https://github.com/twisted/towncrier/commit/5bf0a4653bb2adf52162550d436f1dc39f6e0446"><code>5bf0a46</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/hawkowl/towncrier/issues/396">#396</a> from twisted/292-more-ci-checks</li>
<li><a href="https://github.com/twisted/towncrier/commit/1bb62219a9e7b0d969292cae56104678b8bea151"><code>1bb6221</code></a> Merge branch 'trunk' into 292-more-ci-checks</li>
<li><a href="https://github.com/twisted/towncrier/commit/07baa2ba4b1159462dcb212395be1de91674698d"><code>07baa2b</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/hawkowl/towncrier/issues/395">#395</a> from twisted/384-cli-help</li>
<li>Additional commits viewable in <a href="https://github.com/hawkowl/towncrier/compare/21.9.0...22.8.0">compare view</a></li>
</ul>
</details>
<br />


You can trigger a rebase of this PR by commenting ``@dependabot` rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- ``@dependabot` rebase` will rebase this PR
- ``@dependabot` recreate` will recreate this PR, overwriting any edits that have been made to it
- ``@dependabot` merge` will merge this PR after your CI passes on it
- ``@dependabot` squash and merge` will squash and merge this PR after your CI passes on it
- ``@dependabot` cancel merge` will cancel a previously requested merge and block automerging
- ``@dependabot` reopen` will reopen this PR if it is closed
- ``@dependabot` close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- ``@dependabot` ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- ``@dependabot` ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- ``@dependabot` ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>

Changes needed for this

* Invert logic for single file. 22.8.0 fixed the logic such that single_file=False is correct when you have more than one file
* Pin versions of towncrier and sphinx towncrier to reflect this (so we get the right config)


Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jens H. Nielsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@alan-geller alan-geller alan-geller approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@giulioungaretti giulioungaretti

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@giulioungaretti @alan-geller