mitiq icon indicating copy to clipboard operation
mitiq copied to clipboard

Update `about()` and document latest package support

Open crazy4pi314 opened this issue 2 years ago • 5 comments

Fixes #1201

Description

Please explain the changes you made here.

Checklist

Check off the following once complete (or if not applicable) after opening the PR. The PR will be reviewed once this checklist is complete and all tests are passing.

  • [ ] I added unit tests for new code.
  • [x] I used type hints in function signatures.
  • [ ] I used Google-style docstrings for functions.
  • [ ] I updated the documentation where relevant.
  • [x] Added myself / the copyright holder to the AUTHORS file

If some items remain, you can mark this a draft pull request.

License

  • [x] I license this contribution under the terms of the GNU GPL, version 3 and grant Unitary Fund the right to provide additional permissions as described in section 7 of the GNU GPL, version 3.

Tips

  • If the validation check fails:

    1. Run make check-types (from the root directory of the repository) and fix any mypy errors.

    2. Run make check-style and fix any flake8 errors.

    3. Run make format to format your code with the black autoformatter.

    For more information, check the Mitiq style guidelines.

  • Write "Fixes #XYZ" in the description if this PR fixes Issue #XYZ.

crazy4pi314 avatar Apr 12 '22 00:04 crazy4pi314

Binder :point_left: Launch a binder notebook on branch unitaryfund/mitiq/crazy4pi314/issue1201

github-actions[bot] avatar Apr 12 '22 00:04 github-actions[bot]

Codecov Report

Merging #1220 (b685e90) into master (f0303d6) will decrease coverage by 0.42%. The diff coverage is 65.67%.

@@            Coverage Diff             @@
##           master    #1220      +/-   ##
==========================================
- Coverage   98.13%   97.71%   -0.43%     
==========================================
  Files          60       60              
  Lines        2737     2796      +59     
==========================================
+ Hits         2686     2732      +46     
- Misses         51       64      +13     
Impacted Files Coverage Δ
mitiq/_about.py 66.66% <65.15%> (+30.30%) :arrow_up:
mitiq/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f0303d6...b685e90. Read the comment docs.

codecov[bot] avatar Apr 12 '22 00:04 codecov[bot]

From #1201 there is still a few things left that this does not do, and I am a bit stuck on.

Add warning to QPROGRAM type builder I am not sure the user experience we are looking for on this/where would be the right place to handle. It seems like it would be too annoying to put anytime a conversion is called, and it would already alert the user at the time of mitiq import. Maybe that is sufficient?

Add latest_supported() with output to docs under Supported Frontends I have exported a function for user use called latest_supported_packages, but code cannot be executed in the readme. If we want that, we could have a script that greps the markdown file for versions and replaces them, but that seems tedious. Anyone have other options/suggestions? I just hardcoded them for now...

crazy4pi314 avatar Apr 18 '22 18:04 crazy4pi314

From #1201 there is still a few things left that this does not do, and I am a bit stuck on.

Add warning to QPROGRAM type builder I am not sure the user experience we are looking for on this/where would be the right place to handle. It seems like it would be too annoying to put anytime a conversion is called, and it would already alert the user at the time of mitiq import. Maybe that is sufficient?

I agree the current approach is good and is sufficient.

Add latest_supported() with output to docs under Supported Frontends I have exported a function for user use called latest_supported_packages, but code cannot be executed in the readme. If we want that, we could have a script that greps the markdown file for versions and replaces them, but that seems tedious. Anyone have other options/suggestions? I just hardcoded them for now...

Fine to hardcode versions or to just remove them. Another option is to call latest_supported() in a myst file of the docs that is different from the readme. But I am not sure if there is a good place for it in the current structure. Maybe somewhere in this page?

andreamari avatar Apr 21 '22 17:04 andreamari

The mypy issues are stumping me, I think I have created a race condition with the package loading process... :(

crazy4pi314 avatar Apr 29 '22 16:04 crazy4pi314

This PR got more involved than expected. I also tried to fix some aspects at some point but the situation is complicated since dev requirements are not easily accessible from pip versions of Mitiq (from PyPy).

Maybe we could close it in favor of a new PR which is focused on the simplified issue #1478.

andreamari avatar Sep 02 '22 17:09 andreamari

Based on the discussion in https://github.com/unitaryfund/mitiq/issues/1478, we can close this PR. See https://github.com/unitaryfund/mitiq/issues/1478#issuecomment-1251454833 therein for a good summary.

andreamari avatar Oct 03 '22 14:10 andreamari