Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moved the metadata into PEP 621-compliant pyproject.toml. #1157

Merged
merged 2 commits into from
Jul 3, 2023

Conversation

KOLANICH
Copy link
Contributor

@KOLANICH KOLANICH commented Jun 1, 2022

No description provided.

@erezsh
Copy link
Member

erezsh commented Jun 1, 2022

Thanks, but I think we'll stick to setup.py until pyproject.toml can extract the version from lark/__init__.py.

@KOLANICH
Copy link
Contributor Author

KOLANICH commented Jun 1, 2022

but I think we'll stick to setup.py until pyproject.toml can extract the version from lark/init.py.

It is a feature not of pyproject.toml, but of build backends and plugins like read_version. setuptools can do it, and this feature is used here. But I think that the version should be extracted from git, not from source files.

@erezsh
Copy link
Member

erezsh commented Sep 11, 2022

@KOLANICH I think it's great that you can do lark.__version__ and get the version, very helpful for debugging. I'm actually annoyed that a lot of packages don't do that these days.

@KOLANICH
Copy link
Contributor Author

setuptools_scm has a feature to generate a python source code file for a version extracted from git tags. I can utilize it instead of fetching version from __init__.py.

@erezsh
Copy link
Member

erezsh commented Sep 11, 2022

That sounds like a roundabout solution, but I'm willing to consider it.

@henryiii
Copy link
Contributor

See https://scikit-hep.org/developer/pep621#versioning for an example with hatchling (which works with setuptools_scm). Most of https://scikit-hep.org/developer/packaging#git-tags-official-pypa-method is valid if you want to stick with setuptools, just ignore the setup.cfg mentions. :)

pyproject.toml Outdated Show resolved Hide resolved
nearley = ["js2py"]
atomic_cache = ["atomicwrites"]

[tool.setuptools]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hatching handles this better, IMO, and works even if some limits setuptools<60 in a non isolated environment (cough, NumPy, cough)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, from what I gather, the next version of setuptools is making tool.setuptools non-provisional.

@KOLANICH
Copy link
Contributor Author

if some limits setuptools<60 in a non isolated environment (cough, NumPy, cough)

I heard something about it (only a bit, I have not yet lurked about the issue), but I think it is an issue of numpy. If some package uses < constraints, it is that package that is broken (it doesn't mean the ones that require <= shouldn't lover the version on RHS of the expression).

You don’t need wheel - it’s added by setuptools via PEP 517 and it’s likely to be going away at some point.

Thanks for noting this, I'll fix it.

@erezsh
Copy link
Member

erezsh commented Apr 26, 2023

Cool, that looks elegant! Do you think the transition will be entirely seamless, or are there any details to be aware of?

@KOLANICH
Copy link
Contributor Author

KOLANICH commented Apr 26, 2023

Do you think the transition will be entirely seamless, or are there any details to be aware of?

It depends on your assumptions on Python and setuptools versions users have and on their workflows. Since setuptools has dropped old Python versions. I mean if we say that users of certain versions and/or certain workflows (i.e. the ones who hardcode python3 ./setup.py in their scripts, now they would have to switch to using python3 -m build) are out of scope, then yes, maybe it can be considered "seamless" for some.

Every change breaks someone's workflow and there is ain't no such a thing as "seamless".

xkcd "Workflow"

@erezsh
Copy link
Member

erezsh commented Apr 26, 2023

This is the first time I hear of python3 -m build.

Is there a guide or tutorial for transitioning from setup.py, for those of us who are out of scope? ;)

@KOLANICH
Copy link
Contributor Author

KOLANICH commented Apr 26, 2023

Here are the docs: https://pypa-build.readthedocs.io/en/latest/

python3 -m build is a backend-neutral way to trigger build (now setuptools is just one of the backends). PEP 517 and PEP 518 are used to specify the backend, be it setuptools, or something else (like flit or hatchling). Those backends just have no setup.py. PEP 621 is just a backend-neutral format for metadata instead of the total mess that happenned after PEP 517 got implemented in pip, when each backend used to have an own format. I cannot say the mess is fully cleaned, but PEP 621 is surely an improvement.

If one wants to migrate other projects, I recommend to do the following 2-step process (I did it myself for all my projects, and also for this one):

  1. use setuptools_py2cfg to move the metadata into setup.cfg. Create a PEP 617-compliant pyproject.toml. Delete setup.py.
  2. use ini2toml to convert the metadata into pyproject.toml. Delete setup.cfg.

After each step manual tweaking of the configuration may be needed and you should be sure that the project builds. It is better to introduce setuptools_scm after the step 1.

@henryiii
Copy link
Contributor

There’s also https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html

https://scikit-hep.org/developer is a good place for modern packaging suggestions. Also I’m fond of hatch new --init, which converts setup.py/setup.cfg directly to hatchling.

Many PyPA packages have moved, like packaging and wheel. So it’s pretty safe.

@henryiii
Copy link
Contributor

And personally I almost always use pipx run build. :)

@erezsh
Copy link
Member

erezsh commented Jun 26, 2023

Hello! Sorry it took me this long to look into this. Thank you for the info and the links. I found scikit hep especially interesting.

I'm generally in favor of the change.

Just one question - if I understand correctly, users are now expected to run "pytest" instead of "setup.py test". Does it mean we should also include pytest in the dev-requirements?

Or is it up to the user to install it, or use pipx run pytest?

@henryiii
Copy link
Contributor

FYI, scikit-hep.org/developer has been mostly migrated to https://learn.scientific-python.org/development/

Yes, setup.py test has been deprecated for years. You do want pytest in dev-requirements (I usually have a [test] extra, so a user can install -e.[test]).

Usually you don't want use pipx for pytest, since pytest imports your package, its requirements, and plugins; it's more like a library than an app. In some simple cases it does work, but it's generally designed to run alongside your package.

@KOLANICH
Copy link
Contributor Author

My approach to testing is writing test suites using vanilla unittest and sometimes with standalone libs not related to pytest (to allow them be run without pytest which is large and slow and a dependency, and pytest CLI tool is smart enough to understand such tests), but to run tests using pytest on CI in order to generate reports (on GitLab CI these reports can be integrated into GUI by pointing the report file in the config). Locally I usually use pytest --lf or just ./tests/tests.py depending on what is faster (pytest initialization is quite slow).

@erezsh
Copy link
Member

erezsh commented Jun 27, 2023

So, everyone okay with merging this as-is? We can add a [test] extra in a separate PR.

@MegaIng Feel free to chime in too, if you have something to add.

@MegaIng
Copy link
Member

MegaIng commented Jun 27, 2023

I don't have a lot of concrete experience with this stuff and therefore don't have anything to add.

I am okay with merging.

@erezsh erezsh merged commit 2a3137b into lark-parser:master Jul 3, 2023
9 checks passed
@erezsh
Copy link
Member

erezsh commented Jul 3, 2023

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants