Metadata Errors and Python Wheel Issue

[walchko]

I wrote this in another issue, adafruit/Adafruit_CircuitPython_LSM6DS#24, but it was suggested I make it its own issue.

• package metadata is wrong, this works on python 3.7 and 3.8, I haven't tested 3.6. Also, isn't this more Beta level or higher?

  classifiers=[
      "Development Status :: 3 - Alpha",
      "Intended Audience :: Developers",
      "Topic :: Software Development :: Libraries",
      "Topic :: System :: Hardware",
      "License :: OSI Approved :: MIT License",
      "Programming Language :: Python :: 3",
      "Programming Language :: Python :: 3.4",
      "Programming Language :: Python :: 3.5",
  ],

• you only make source packages on pypi, not wheels ... why?

  • I can see you are trying to build wheels here, but it isn't getting pushed to pypi

  - name: Build Python package
    run: |
      pip install --upgrade setuptools wheel twine readme_renderer testresources
      python setup.py sdist
      python setup.py bdist_wheel --universal
      twine check dist/*

  • I am unfamiliar with the --universal ... never used that before
  • Also, I have switched to poetry and just use a pyproject.toml which holds everything now ... it is awesome
    • Oh, I have no affiliation with poetry other than I really like it
• There is the temperature sensor access, but I have that under a different issue ticket as an enhancement

[siddacious]

Thanks for creating this issue @walchko; I've moved it to the cookiecutter repo because it's the template we use for all of the libs, so the setup.py gets generated from here: https://github.com/adafruit/cookiecutter-adafruit-circuitpython/blob/master/%7B%7B%20cookiecutter.library_name%20%7D%7D/setup.py#L63

@theacodes can you give your input here? I'm a total newb when it comes to python packaging

[sommersoft]

While I wasn't personally involved in setting up the initial setup.py that is in use for PyPI'd CircuitPython libraries, I'll try and add some of the "why"s for the classifiers. @kattni, please correct me if I'm wrong on any of this speculation.

• Development Status : : 3 - Alpha: since this cookiecutter is aimed at brand new libraries, they essentially start at an Alpha level. I'd agree that they should be updated as they mature, but I'm not sure if that happens. There is probably an opportunity here for some automated checking/updating, since Semantic Versioning is the standard.
• Programming Language :: Python :: 3, 3.4, 3.5: I think these were chosen since these libraries are written primarily for running on microcontrollers with CircuitPython, which as a fork of MicroPython, is based on CPython 3.4/3.5. They should not really operate with the expectation that features in 3.6 and above will be available. I also am not super well versed on proper Python packaging, so I'm not sure if this has any negative effects on target environments with new versions.

[walchko]

So, I am not sure there is any impact of showing older python versions in the metadata, but here in your gitworkflow, you are specifically pulling python 3.6 to run your tests.

Also, I made a mistake, when your code here builds the source and wheel package, it only checks if the readme will display correctly on pypi ... it doesn't push it to pypi. You do that in release.yml here, but you only do: python setup.py sdist. You could change that to python3 setup.py sdist bdist_wheel and build both wheels and source. Just a suggestion. :smile:

[2bndy5]

The cookiecutter repo only calls setup.py sdist in the release.yml Technically, @walchko is correct: the python setup.py bdist_wheel --universal call doesn't need to be in the build.yml file. So this issue is actually 2 issues wrapped in 1.

About using setup.py bdist

See PyDocs about using wheels for install. @walchko the --universal option is documented in the PyDocs

To my understanding, the bdist option is meant for packages that:

1. take an unusually long time to build. The package would have to be VERY large (consisting of many many modules) for this to become a problem.
2. aren't all pure python. Say you have a package that's wrapping some C++ using pybind11 (or one of the various extensions that implement Python's C API), building/uploading wheels to pypi is required, so that pip can determine which binary wheel to install (based on the local machine's OS/arch bitness/Python Version/etc). Furthermore, distributing wheels in this situation relieves pip on the target machine of having to make sure the right version of gcc (and possibly boost.python) is installed.

Because CirPy firmware does not support extension packages that are written in C++ and typical CirPy packages are relatively small/quick to install, I'm not sure uploading binary wheels to pypi would be valuable (it might only complicate things for distribution).

• I've been looking into rewriting the python wrapper for TMRh20's RF24 libraries, and that research is where my understanding of uploading wheels to pypi comes from. If there is more nuance than my seemingly limited understanding, please correct me on this.

About the classifiers list

The items in this list are only used to filter searching/browsing on pypi.org. They don't actually stipulate hard requirements for package installs/execution. (see PyDocs for more detail)

• Personally, I think the "Development Status" tag could use a # TODO-like inline comment since it currently requires library maintainers to alter it accordingly (newcomer or not). Of course, sphinx's todo_emit_warnings option needs to be switched off to avoid doc building errors if the inline comment actually includes TODO. It doesn't need to be a proper # TODO comment. e.g. # remember to change this when the library is mature.
• I fully endorse bumping the minimum supported Python version to v3.7 because (as @siddacious mentioned in #85 ) time.monotonic_ns() was introduced in v3.7 while CirPy firmware has had time.monotonic_ns() for quite some time (iirc since v4.0). Furthermore, the latest release of Raspberrry Pi OS ships with Python v3.7.2. Note that the "Programming Language :: Python :: 3" tag can stay as it helps broaden searching/browsing filters on pypi.
  • The proper way to indicate minimum Python version as a hard requirement is via the python_requires option in setup.py (docs say that this implementation requires pip version 9.0.0+).

About pyproject.toml file

This file (introduced as part of PIP518) basically tells pip to create a temp venv for building the package during install. It is meant as an alternative to putting build-time-only dependencies in requirements.txt file. After install, the temp venv is destroyed. CirPy packages shouldn't need any extra build-time dependencies, especially for CirPy firmware compatible packages. Currently, this repo uses pyproject.toml to tell the black linter what Python version to support (I think)

@walchko I've never heard of the "poetry" project, but it does look interesting. Thanks for the lead! FYI, the pyproject.toml isn't specific to the peotry project, and I think some of the poetry project's usage in pyproject.toml may be specific to the poetry project.

[tekktrik]

With the migration to pyproject.toml, Python wheels are being built and uploaded, and the classifiers have been updated.