BoothGroup / Vayesta

A Python package for wave function-based quantum embedding
Apache License 2.0
33 stars 8 forks source link

Adds bare run, drops 3.7, adds 3.11 to CI #104

Closed obackhouse closed 1 year ago

obackhouse commented 1 year ago

The CI did not include a run that builds with none of the optional dependencies. This PR changes the standard runs to now use all optional dependencies, and adds a 'bare' run with none of them. It also drops 3.7 and adds a 3.11 run.

obackhouse commented 1 year ago

FYI @cjcscott I think that ebcc wasn't actually getting imported in the CI on #92 :grimacing:

cjcscott commented 1 year ago

I thought we already knew this? Hence lots of skipped tests? Good to run them though - 3.11 a good addition as well, let's see if any bugs gets turned up!

obackhouse commented 1 year ago

Ah I may have misunderstood, did the ebcc tests pass locally? Should be fine if so.

obackhouse commented 1 year ago

I've also edited this PR to drop CI support for python 3.7. It's EOL is in 3 weeks and dyson specifically requires >=3.8 anyway. I won't change the Vayesta requirements to disallow 3.7 for now, but I don't see much point in CI/CD for python versions past their EOL.

cjcscott commented 1 year ago

Yeah, I've run tests with ebcc separately when plumbing in the interface etc. No opposition to dropping python 3.7, given eol sounds very sensible!

cjcscott commented 1 year ago

I'll check what's happening with cvxy as a dependency here, the nicely skipping tests functionality appears to not be working..

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +0.08 :tada:

Comparison is base (10db358) 71.99% compared to head (9601255) 72.07%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #104 +/- ## ========================================== + Coverage 71.99% 72.07% +0.08% ========================================== Files 138 138 Lines 18402 18499 +97 Branches 2578 2976 +398 ========================================== + Hits 13248 13334 +86 - Misses 4424 4427 +3 - Partials 730 738 +8 ``` [see 11 files with indirect coverage changes](https://app.codecov.io/gh/BoothGroup/Vayesta/pull/104/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BoothGroup)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

obackhouse commented 1 year ago

I'll check what's happening with cvxy as a dependency here, the nicely skipping tests functionality appears to not be working..

I've had trouble getting it to play nicely inside unittest.TestCases before, never quite worked out why. I think you could also do

if "cvxpy" not in sys.modules:
    pytest.skip("Requires cvxpy")

inside the function body (or the setUpTests maybe? Maybe not)

obackhouse commented 1 year ago

I fixed the cvxpy stuff, this can be merged