Modernize DevOps Tooling

[coltonbh]

Description

Modernize DevOps tooling. No changes to code functionality in qcel, only devops tooling.

Changelog description

• Added /scripts directory to root of project that contains scripts for testing, formatting code, and building docs.
• Updated build system from setuptools to modern pyproject.toml specification using poetry for the build backend.
• Removed complicated versioning code in favor of single source of truth in pyproject.toml. Using standard library importlib for looking up package version in __init__.py file.
• Added build_docs.sh script to /scrips and removed Makefile from /docs. Flattened /docs file structure.
• Removed travis-ci code from devtools
• Removed LGTM code (they no longer exist as a project).

TODO

• Figure out what the pytest --validate thing was about. @loriab said this was OK to drop.

Did Not Do

• Test currently write scratch data to the repo in to qcelemental/tests/qcshema*/*.json. The tests rely upon /qcschema/{NAME}/ already being created, so these currently junk up the repo with a dummy file in them so git will keep the empty folders in the repo. Modify tests to remove these unnecessary directories to clean up the repo. Test should never write data to the repo. Junk data should be written to /tmp or some other temporary in memory buffer. If we want this output data for something (like QCSChema examples) we should have a scripts/build_examples.sh script, not couple the task to our test. Separation of concerns :)
• Tests that call PubChem should not. Tests should never rely upon external services (like web APIs). We should monkeypatch http calls to return the data as if PubChem were returning it.

Notes

• The max version limitations in dependencies are due to packages no longer supporting python3.7, so we have to specifiy max versions. We can remove these limitations once python3.7 reaches end of life June 27, 2023.

Status

• [x ] Code base linted
• [ x] Ready to go

[codecov]

Codecov Report

Merging #310 (d7be98b) into master (86b4700) will increase coverage by 2.55%.
The diff coverage is 93.33%.

❗ Current head d7be98b differs from pull request most recent head ab6e830. Consider uploading reports for the commit ab6e830 to get more accurate results

Additional details and impacted files

[loriab]

Moved all tests to /tests directory in the root of the project so we no longer distribute test code with built packages.

I've generally considered distributing tests to be a good thing. Especially as software becomes more modular, it can be helpful to test packages in situ. And I suspect packagers want to run tests as part of build checks. Other views?

Also, I noticed a lot of new docs/api/ files. I'm pretty sure these are autogenerated by the sphinx build, so they don't need to be stored.

[coltonbh]

Good catch on the /docs/api! I removed them and updated the .gitignore.

Re: tests. Testing is a development concern, not an end-user concern. End users should never need to run tests on a distribution. The presumption is that distribution has already been tested before the maintainers (us) distributed it. We aren't shipping built packages that weren't tested are we? ;P

So test code is extra cruft in a distribution that adds weight and provides no value (the exact code they are running already was tested!). In a world where people are compiling complicated code and bugs may be strange library compatibility issues (like old scientific C librararies) it makes sense to distribute test for santity. But this is because end users are also building. With python, there is no build step. The code is good-to-go as long as we tested it first.

Developers can run tests just like always with bash scripts/test.sh or just running pytest from the root of the repo. But distributed packages (those on pypi) should never have test code with them.

[coltonbh]

As examples take a look at FastAPI, pydantic, flake8, poetry, basically any standard python pacakge, /tests lives in the root, and it is not packaged with distros :)

[loriab]

I can see the user-concern/developer-concern distinction -- certainly I wouldn't run MS Word tests, and I've maybe run Python tests once. But those aren't so close to research and probably get much broader testing at release time than qca.

Sure, qcel is tested before pkg building :-) For my part, it's likely even been tested a couple layers downstream through qcng, psi4, qcdb (nwc, gms, cfour). But that's still not great environmental coverage. For example, devs test may on Linux, but maybe we don't generalize the paths right, so something breaks for the user on Windows. Or devs test with MKL, but users run with OpenBLAS, or worse half-MKL, half-OpenBLAS (thanks, pypi). Or devs test with latest and min-version dependencies, and an unintentional breaking change is released for a dep the next day. Or devs test with released versions and users have development versions of several packages. Users are great at installing software combos that faintly horrify developers.

Most of these troubles do involve complications from the compiled code and library compatibility tangles you mentioned, not qcel itself. And some of these arguments apply less strongly to qcel than to qcengine, which is literally tying together these unwieldy projects that never expected to bump shoulders. But tests are fairly light and a great way to diagnose package vs. environment problems. Since qcarchive packages are closer to research than broader tools like fastapi, numpy is probably a better model, and it looks like it packages its tests.

[coltonbh]

I hear you :)

A few additional thoughts:

1. numpy packages its tests because there is a build step associated with installing it (at least if you use a srcdist instead of a wheel). You have to perform a compilation step with numpy, and that can have issues (MKL/BLAS stuff is common, as you point out). Packages that require builds using local compilers are a separate class of code. We are pure python.
2. I'd like to change your perspective that qcel is research code! It's a very basic data structures package in pure python. Sure, it's used in research--but this isn't research code. It's a very standard python library. It should be a robust, stable package. :)
3. Re: your points above, if something is broken for end users as you mention--like a Windows user--having tests on their machine won't help anything. The package already broke for them and they have the corresponding stack trace for opening an issue. Tests will not run either, for the same reason. What's the value that running broken tests adds? Breaking changes in newly released dependencies is our fault--it means we didn't specify dependencies correctly in the pyproject.toml. The ^1.3.4 notation you see in there now means only major version 1.x.x allowed, so it avoid the broken scenarios where a new 2.0.0 version breaks code. Again, running tests can't help these end users--test are broken too in this case. As for mixed dependencies, that's on package managers like poetry, pipenv, etc. to root out--presumably if the versions are such that the package doesn't work, the test won't work either because they use the package! :)
4. Lastly, imagine how end users would run test. The pip install qcelemental and they get test code. But can they run it? No, because the testing dependencies are not installed. So now we're distributing non-running code. How do they discover dependencies for tests? They'd look at pyproject.toml in the git repo. How would they install them? pip install pk1, pk2, etc.. Or more simply, they'd do this:

   git clone qcelemental-git-url.ssh
   poetry install
   pytest

   End users can always run tests. And the easiest, fastest way to run them is to clone the repo, install all deps for the package and testing, and then run the tests. Much easier than pip install qcelemental, then manually adding all the deps for tests, then running the tests from your pip installed qcel, which most users probably won't even know where it is (would need to run pytest /path/to/some/python/lib/python3.7/site-packages/qcelemental and I doubt most users will be able to track that down.

That said, you are the boss on this! If you want tests in the distributions, I'll add them back.

I would propose the follow solution as I think it provides the best end-user experience for what you have in mind. Though again, if their package is broken the tests can't run--so I don't think we're really offering anything ;)

1. I'd change all testing dependencies as extras package dependencies so end users can pip install qcelemental[test] and get the test code and required dependencies to run.
2. End users still need to know where their qcelemental package lives. They can find it with the following:

   import qcelemental
   print(qcelemental.__file__)

   Then they can run the tests with pytest /path/to/qcelemental/install/location

What would you like done?

[loriab]

Thanks for all the modernizations. This is a partial review with a few questions.

[loriab]

lgtm

[loriab]

these perhaps stay, too, since the functions do.

[coltonbh]

@loriab updates made. Test passing! Review pls?

[coltonbh]

Woot woot! <3