improving get_version for Molpro and Q-Chem

[loriab]

Description

Changelog description

• at least won't throw xml errors in the presence of an old molpro when probing presence/version
• modernize Provenance for Q-Chem (have to cheat for qcer tests)
• for mp2 energies, get Q-Chem passing stdsuite
• switch to extracting Q-Chem version from output file, not qchem -h since the last isn't available from a source install
• catch up on isort
• lower qchem requirements to v5.1 (works at GT). former v5.2 was simply the version at which the harness was developed.

Status

• Code base linted
• Ready to go

[lgtm-com]

This pull request introduces 1 alert when merging 0003e40 into 7bd2521 - view on LGTM.com

new alerts:

• 1 for Unused import

[codecov]

Codecov Report

Merging #261 into master will increase coverage by 4.56%.
The diff coverage is 47.16%.

[dotsdl]

Looking good! Looks like a lot of the changes are lints, which isn't a problem. The meat of the changes look good to me!

[dotsdl]

Perhaps remove this one since we have the above now?

[loriab]

the found()s? second is normal behavior and has the raise_error behavior. first is the crazy escape path for qcer.

[dotsdl]

But will the second ever have any effect? If qchem isn't found, the first will return the safe_version("0.0.0") call and exit the method. If qchem is found, it will sail through? I might be misunderstanding the subtleties of qchem here...

[loriab]

yes, you're right. the return v0.0 was newly needed to appease qcer after I made qchem run for version. but the second with raise_error is the correct harness behavior. proper solution would be for the qcer tests to set an envvar that could be used to narrowly trigger the v0.0 special behavior. I'd like to keep the second found in memory of what we should return to.

I don't fully understand the subtleties of the qchem harness myself. it's one of those programs that settled into an idiosyncratic groove long ago, and it looks like DGAS had to add extra hoops to to stuff it into a harness.