search

Solid-Energy-Systems / NewareNDA

Python module and command line tool for reading and converting Neware nda and ndax battery cycling data files.
BSD 3-Clause "New" or "Revised" License
15 stars 8 forks source link

Change from setup.py to pyproject.toml #78

Closed Grimler91 closed 2 months ago

Grimler91 commented 2 months ago

I am a creature of habit. Back in the day a setup.py project was suppose to installed with python setup.py install, but running it now on somewhat modern installations give a deprecation warning:

/usr/lib/python3.12/site-packages/setuptools/_distutils/cmd.py:66: SetuptoolsDeprecationWarning: setup.py install is deprecated.
!!

        ********************************************************************************
        Please avoid running ``setup.py`` directly.
        Instead, use pypa/build, pypa/installer or other
        standards-based tools.

        See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
        ********************************************************************************

!!

Proper way to install projects is the one specified in README: pip install ., but let's anyways convert to a pyproject.toml so that there is zero chance of myself and other users using the deprecated installation command to install NewareNDA.

The NewareNDAcli script installs to NewareNDA-cli.py as before (but seems like it - is not allowed in filename so had to rename the file), and pytest still works.

d-cogswell commented 2 months ago

This is a nice improvement! I had to make a few changes to get it to install. Let me know if they're ok.

Grimler91 commented 2 months ago

@d-cogswell Hmm, what issues did you have with the version number in original attempt?

Thanks for spotting duplicate readme entry! I can confirm that it installs and seem to work fine with your changes as well!

d-cogswell commented 2 months ago

Hi @Grimler91 your original version line was causing this error for me on a couple different Python versions: ValueError: invalid pyproject.toml config: `project.version`. configuration error: `project.version` must be string

I couldn't figure out exactly what it didn't like, but the dynamic option seems to work.

Grimler91 commented 2 months ago

Rebased against development branch, should be ready to be merged IMHO, thanks!