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

Feature/export png with dpi #139

Merged
merged 14 commits into from
Apr 30, 2022
Merged

Conversation

srinathkp
Copy link
Contributor

@srinathkp srinathkp commented Apr 25, 2022

For #133 - Allow setting dpi/density when exporting circuits to PNG

New method QubitCircuit.draw() to export svg and png images. Density in dpi can be an argument from the user for conversion to png ( defaults to 100dpi).

@BoxiLi
Copy link
Member

BoxiLi commented Apr 26, 2022

Looks good. I'll test it locally later today. The falling linting is just the black python styling.

We don't really want to include ipynb files in the repo because they are binary and are pretty large. They won't be automatically executed by pytest anyway.

Instead, you could add a test to test_circuit.py. If pdflatex is not installed, the test should be ignored. Otherwise, the test checks if a plot is successfully generated. What do you think?

@srinathkp
Copy link
Contributor Author

srinathkp commented Apr 26, 2022

  • Add test to py_test.py
  • Remove ipynb file
  • Add density_dpi argument to old _make_converter to avoid duplication

@srinathkp
Copy link
Contributor Author

Hi @BoxiLi , I've added the density argument to old converter and added a test that looks for exported png files(with different dpi) and SVG.

Copy link
Member

@BoxiLi BoxiLi 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! Thank you!
I left some comments to clear the logic in the code. Don't worry about the coverage, the CI has no pdflatex installed.

src/qutip_qip/circuit.py Outdated Show resolved Hide resolved
src/qutip_qip/circuit.py Outdated Show resolved Hide resolved
src/qutip_qip/circuit.py Outdated Show resolved Hide resolved
src/qutip_qip/circuit.py Outdated Show resolved Hide resolved
src/qutip_qip/circuit.py Outdated Show resolved Hide resolved
@BoxiLi BoxiLi linked an issue Apr 29, 2022 that may be closed by this pull request
Copy link
Member

@BoxiLi BoxiLi left a comment

Choose a reason for hiding this comment

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

Tested it locally, LGTM!

@BoxiLi BoxiLi merged commit 6dc4415 into qutip:master Apr 30, 2022
@BoxiLi BoxiLi added this to the qutip-qip-0.2.2 milestone Apr 30, 2022
@BoxiLi
Copy link
Member

BoxiLi commented Apr 30, 2022

@srinathkp Congrats!

@srinathkp
Copy link
Contributor Author

Thanks Boxi ! Many more to come hopefully 💥🤞

@srinathkp srinathkp deleted the feature/export-png-with-dpi branch May 1, 2022 17:32
BoxiLi pushed a commit to BoxiLi/qutip-qip that referenced this pull request Jun 18, 2022
New method QubitCircuit.draw() to export svg and png images. Density in dpi can be an argument from the user for conversion to png (defaults to 100dpi).
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.

Allow setting dpi/density when exporting circuits to PNG.
2 participants