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

Code quality and pylint improvements to A #11

Merged
merged 139 commits into from
Apr 16, 2022
Merged

Code quality and pylint improvements to A #11

merged 139 commits into from
Apr 16, 2022

Conversation

nariman87
Copy link
Contributor

@nariman87 nariman87 commented Apr 6, 2022

Context for changes

  • Improvements:
    • A large number of linting corrections were made to improve the overall pylint report. These were mostly minor but essential modifications including restructuring, re-coding, optimization, updating .pylintrc, adding .coveragerc. The code quality score is improved to A for the released version. Check "Files changed" for details.
    • TODO comments have been removed exclusively from files with low code quality grades. The Dev team has created tickets to be actioned for all removed TODO comments on separate (private) FlamingPy boards.
    • CONTRIBUTING.rst, code_of_conduct.md, and CHNAGLELOG.md were imported and modified from the StrawberryFields project. Dev team plans to extend these with customized details in future PRs.

Example usage and tests

  • Not Applicable

Performance results justifying changes

  • Not Applicable

Workflow actions and tests

  • Not Applicable

Expected benefits and drawbacks

Expected benefits:

  • Improving overall code quality score from B to A. Note the acceptable limit for all future PRs is now set at A.

Related Github issues

  • Not Applicable

Checklist and integration statements

  • My Python and C++ codes follow the coding and commenting styles of this project as indicated by existing files. Specifically, the changes conform to given black, docformatter and pylint configurations.
  • I have performed a self-review of these changes. I have checked my code and corrected misspellings to the best of my capacity. I have checked the codefactor score in the active branch and ensured it is A- or better. I also confirm that I have already merged other branches into this branch as required.
  • I have added context for corresponding changes in documentation and README.md as needed.
  • I have added new workflow CI tests for corresponding changes and these pass locally for me.
  • I have updated CHANGELOG.md following the template. I recognize that the developers may revisit CHANGELOG.md, the versioning, and create a Special Release including my changes.

nariman87 and others added 4 commits April 3, 2022 20:24
* Update _version.py

* Add IID Pauli noise model

* Remove unused imports

* Fix issue with linting

* Add iid noise example to decoder.py

Allow user to change between cv and dv noise examples.

* Run black

* No need to specify inner decoder for iid case

* Run black on test_iid_noise

* Remove uniform weight assign

* Fix typos and change data function

* Fix test docstring

* Improve test

* Add docstring to apply_noise

* Update flamingpy/decoders/decoder.py

* Update iid.py

* Update flamingpy/noise/iid.py

* Delete pauli.py

* Update _version.py

* Update docs; run black

* Update _version.py

Co-authored-by: Narimanium <25132802+nariman87@users.noreply.github.com>
Co-authored-by: maxtremblay <m@xtremblay.ca>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>

Co-authored-by: Maxime Tremblay <52462375+maxtremblay@users.noreply.github.com>
Co-authored-by: maxtremblay <m@xtremblay.ca>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #11 (e42bc11) into main (b905698) will increase coverage by 2.18%.
The diff coverage is 97.11%.

@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
+ Coverage   91.57%   93.75%   +2.18%     
==========================================
  Files          32       32              
  Lines        1863     1841      -22     
==========================================
+ Hits         1706     1726      +20     
+ Misses        157      115      -42     
Impacted Files Coverage Δ
flamingpy/__init__.py 94.28% <ø> (+2.85%) ⬆️
flamingpy/codes/surface_code.py 95.85% <95.83%> (-0.84%) ⬇️
flamingpy/cv/ops.py 89.01% <97.50%> (+1.90%) ⬆️
flamingpy/_version.py 100.00% <100.00%> (ø)
flamingpy/cv/macro_reduce.py 96.20% <100.00%> (ø)
flamingpy/examples/decoding.py 87.50% <100.00%> (ø)
flamingpy/examples/gkp.py 100.00% <100.00%> (ø)
flamingpy/utils/viz.py 87.71% <100.00%> (+0.10%) ⬆️
flamingpy/benchmarks/matching.py 100.00% <0.00%> (+2.32%) ⬆️
flamingpy/benchmarks/decoding.py 100.00% <0.00%> (+2.63%) ⬆️
... and 4 more

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 b905698...e42bc11. Read the comment docs.

@nariman87 nariman87 marked this pull request as draft April 6, 2022 07:44
@nariman87 nariman87 self-assigned this Apr 6, 2022
@nariman87 nariman87 added the chores Standard routines for open-source software projects label Apr 6, 2022
nariman87 and others added 2 commits April 16, 2022 11:50
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
README.md Outdated Show resolved Hide resolved
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
README.md Outdated


## Contribution

We welcome new contributions -- simply fork the FlamingPy repository and make a pull request (PR) containing your contribution. All contributors to FlamingPy will be listed as authors on the releases. Users who contribute significantly to the code (new plugins, functionalities, etc.) may be listed on the arXiv preprints for FlamingPy. See our [release notes and changelogs](https://github.com/XanaduAI/flamingpy/releases) for more details.
[See our CONTRIBUTING.rst](.github/CONTRIBUTING.rst) <!-- to be changed from relative paths to links -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we / should we have the full text appear here, like it does in the SF readme?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not possible, unfortunately, see this comment:

This is not possible to my knowledge and will not work. GitHub md does not support include rendering at all, not even for rst!!! And will likely never do. See here. There are third-party tools to merge files before sending them to GitHub (e.g. https://github.com/BurdetteLamar/markdown_helpe), but the results are the same, you have to manually render the file every time.

The best workaround currently is to include a link to the centralized file we want to keep. I just implemented that: so README and docs are linking to .github/CONTRIBUTING.rst (had to replace the .md with .rst for proper rendering in docs). How's that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to either copy-paste manually or run a tool every time or add a link, which is a better solution in my view.

tests/unit/test_graphstates.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
flamingpy/cv/ops.py Outdated Show resolved Hide resolved
nariman87 and others added 15 commits April 16, 2022 12:48
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
@nariman87 nariman87 requested a review from ilan-tz April 16, 2022 03:06
@ilan-tz ilan-tz merged commit caf8d47 into main Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chores Standard routines for open-source software projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants