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

Localise and decrease openfermion-dependency in qchem tests #2593

Merged
merged 11 commits into from
May 26, 2022

Conversation

soranjh
Copy link
Contributor

@soranjh soranjh commented May 19, 2022

Context:
This PR localises the openfermion-dependent tests in one place and reduces openfermion dependency in qchem tests where possible.

Description of the Change:

  • All openfermion-dependent tests are now located in of_tests.

  • The new module test_structure is created to collect the tests of the qchem.structure module in one place. These tests were previously inside of_tests and had some openfermion dependency which is now removed.

  • The rest of the qchem tests are brought inside tests.qchem.

Benefits:
Makes it easier to manage the external dependencies in the qchem tests.

Possible Drawbacks:

Related GitHub Issues:

@soranjh soranjh added WIP 🚧 Work-in-progress qchem ⚛️ Related to the QChem package labels May 19, 2022
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented May 19, 2022

Codecov Report

Merging #2593 (70fbcaf) into master (5d5181f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2593   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files         243      243           
  Lines       19630    19630           
=======================================
  Hits        19550    19550           
  Misses         80       80           

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 5d5181f...70fbcaf. Read the comment docs.

@soranjh soranjh changed the title [WIP] localise openfermion-dependent qchem tests Localise and decrease openfermion-dependency in qchem tests May 20, 2022
@soranjh soranjh marked this pull request as ready for review May 20, 2022 13:52
Copy link
Contributor

@obliviateandsurrender obliviateandsurrender 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! Just left two unrelated comments. Also, any idea why doc build is failing? 🤔

Found out it is because of protobuf 4.21.0.

tests/qchem/test_structure.py Show resolved Hide resolved
tests/qchem/test_structure.py Show resolved Hide resolved
@soranjh
Copy link
Contributor Author

soranjh commented May 26, 2022

Also, any idea why doc build is failing?

It is passing after a rerun.

@soranjh soranjh merged commit 5c25a25 into master May 26, 2022
@soranjh soranjh deleted the qchem_localize_OF_tests branch May 26, 2022 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qchem ⚛️ Related to the QChem package WIP 🚧 Work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants