open-dis-python icon indicating copy to clipboard operation
open-dis-python copied to clipboard

Issue partial fix for #44: init pyproject.toml and poetry.lock

Open scottrbrtsn opened this issue 1 year ago • 8 comments

for https://github.com/open-dis/open-dis-python/issues/44

Problem

There is a desire to become a pip package per Issue 44 linked above. It is recommended to also include a pyproject.toml so package managers like poetry can resolve dependencies. I recognize this is rather trivial to add for open-dis-python seeing as how (after I looked into it) there are virtually no external dependencies to create the package (only numpy. Not having a pyproject.toml could affect applications downstream desiring to easily add and manage opendis as a dependency.

However, there is concern with explicitly requiring python versions.

Solution

This PR initializes a pyproject.toml, a poetry lock (for the .venv) and sets the pyproject to use python 3.5 or higher.

Still unresolved

The issue in question could likely:

  1. remove the setup.py
  2. follow a pypi publish guide or with poetry publish
  3. then update the README to simply pip install opendis instead of git clone and pip install .

scottrbrtsn avatar Mar 01 '24 01:03 scottrbrtsn

Ah, I'm using my system python... tried on fresh machine and there are dependencies:

The current project's supported Python range (>=3.5,<4.0) is not compatible with some of the required packages Python requirement:

  • numpy requires Python >=3.9, so it will not be satisfied for Python >=3.5,<3.9

From RangeCoordinates

scottrbrtsn avatar Mar 01 '24 02:03 scottrbrtsn

Per https://github.com/numpy/numpy/releases/tag/v1.18.4, the latest version of numpy that supports Python 3.5 is v1.18.4, we'll have to version-lock that dependency to maintain Python 3.5 support

ngjunsiang avatar Mar 01 '24 08:03 ngjunsiang

Still unresolved

The issue in question could likely:

  1. remove the setup.py
  2. follow a pypi publish guide or with poetry publish
  3. then update the README to simply pip install opendis instead of git clone and pip install .

I'm good with these suggested changes as well. Let's move over values from the setup.py file into the new pyproject.toml file so they match as much as we can. Version, description, etc

leif81 avatar Mar 01 '24 12:03 leif81

Oldest python version available on my machine appears to be 3.6: https://wiki.archlinux.org/title/python

3.6 and 3.7 are tagged with "unmaintained". Seeing as how this is a DoD spec'ed protocol, shouldn't the community strongly consider upgrading?

https://www.cvedetails.com/vulnerability-list/vendor_id-16835/product_id-39445/Numpy-Numpy.html

scottrbrtsn avatar Mar 01 '24 13:03 scottrbrtsn

Another good point @scottrbrtsn It is becoming an increasingly good case to set the baseline to Python 3.9. I support that direction now. If there any concerns please leave a comment here.

leif81 avatar Mar 01 '24 13:03 leif81

Looking good.

Is there any benefit to keeping the setup.py if the use of poetry is merged here?

leif81 avatar Mar 02 '24 12:03 leif81

Looking good.

Is there any benefit to keeping the setup.py if the use of poetry is merged here?

I'm not 100% on details. Just that it would affect pip install behavior and folks might need adjust accordingly.

Similar example: https://github.com/wearepal/ranzen/pull/5

References: https://github.com/python-poetry/poetry/issues/321

scottrbrtsn avatar Mar 02 '24 18:03 scottrbrtsn

Ah, this can be tested with:

pip install git+https://github.com/scottrbrtsn/open-dis-python@pyproject-poetry-init

scottrbrtsn avatar Mar 04 '24 14:03 scottrbrtsn