zdelrosario / py_grama

Implementation of a grammar of model analysis
https://py-grama.readthedocs.io/en/latest/
MIT License
19 stars 11 forks source link

Clarity & Testing around supported Python Versions #186

Open mstites opened 2 years ago

mstites commented 2 years ago

Currently py_grama just specifies Python requirement being anything >= 3.6. As identified in issue #185, 3.10 though meeting the official requirements, does not work with all the req-extras. Current testing is only performed on Python 3.8, meaning the other versions of Python marked as supported are not being tested and this could lead to unexpected incompatibilities and confusion.

Things to address:

  1. Identify versions of Python to officially support. This could include identifying versions of Python that will probably work but aren't fully tested (e.g. earlier versions of Python 3). It may be a good idea to test more Python versions for existing compatibility.
  2. Add all officially support Python versions to the GitHub action.
  3. Clearly document which versions of Python are officially supported. This should (probably) include the wiki, README, and updating the evc-course tutorial site setup instructions (which already differs from the grama setup.py).

Python 3.8 and 3.9 have been confirmed working. 3.10 does not work properly with all the req-extras, but may work if the pymatgen requirement can be easily updated. 3.10 appears to work with the default requirements.

Once this is addressed, it would also be a good idea to add test coverage for different system configurations as well (e.g. other Ubuntu versions, osx, Windows).

mstites commented 2 years ago

I wonder if it would be a good idea to apply a quick patch to:

  1. Update the setup.py to exclude 3.10.
  2. Add a note to current supported Python versions on the README. And that we are currently wworking on it.

@zdelrosario Thoughts? Or would it be better to just take more time and get it all done at once?

mstites commented 2 years ago

Research & Suggestions:

I looked at Pandas, Plotnine, Scipy, Pymatgen, and Numpy to get an idea of what some other Python Libraries (of various scales) used for testing. There seemed to be some good amount of variability, but all of these libraries appear to be regularly testing three different python versions. At the very least 3.8 and 3.10 were tested, but there was some variation here (Pandas also tested 3.9.7 and 3.11 (alpha). Numpy also did tests on 3.11).

All except plotnine tested on Ubuntu, Mac, and Windows. These were generally the latest version, except some of the bigger libraries also tested on 18.04 LTS. Of note is that some of the packages (notably Pandas) divided the tests up a fair amount more than we currently do, giving more modularity (i.e. "mac" "windows" and "ubuntu" files, then seperate files to define different tests to iterate through different python versions). This allowed for different lint testing from "release" testing (more thorough testing involving multiple OSes and more python versions that appears to be for releases).

From this, my thought is that the current scale of the project we should implement testing on multiple Python versions, as this doesn't seem hard. Based off of what I have seen so far, we should definetly test on 3.8 & 3.10. I didn't see as much testing on early Python 3 versions, but we should look into that more before ruling that out.

I don't see any reason we would need to create any special release only testing at this point. Our tests aren't likely to be so lengthy / thorough as to take a super long time and it is probably best if we thoroughly verify compatibility along the way given the limited number of contributors.

Testing on Windows and Mac OS is definetly good in the long term. But based on how most of the tests performed for PRs to this library appeared to be Ubuntu only (multiple py. versions), I don't think that should be a high priority at the current stage of development.

mstites commented 2 years ago

@zdelrosario Is this an issue we could close soon? I believe we have established what the supported versions are (3.8, 3.9) and should be (3.8, 3.9, 3.10). Could we just add this info to the README and call this closed for now?