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

Support [project] table in pyproject.toml (PEP 621) #393

Merged
merged 28 commits into from Mar 15, 2021
Merged

Conversation

takluyver
Copy link
Member

@takluyver takluyver commented Feb 27, 2021

Adding support for PEP 621 (closes #377).

You should either use a new-style [project] table, or the existing [tool.flit.metadata] format: Flit won't allow you to mix them. Version and description may be given statically in the [project] table, or listed in the dynamic= field to use Flit to get them from the module as before.

Todo:

  • Allow specifying a name for the importable module/package (the default is the same as the distribution name)
  • Normalise version when statically specified

Maybe later:

  • Documentation
  • Generate the new format in flit init
  • Conversion utility like flit.tomlify?

(I'm thinking I might release it and let enthusiastic users try it out before encouraging everyone to use it)

Copy link

@pyfisch pyfisch left a comment

Choose a reason for hiding this comment

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

Executing ./bootstrap_dev.py causes an error in common.py:

INFO:flit.install:Extras to install for deps 'all': {'.none'}
INFO:flit.install:Installing requirements
Requirement already satisfied: toml in /usr/lib/python3.9/site-packages (from -r /tmp/tmpmf694e1xrequirements.txt (line 1)) (0.10.2)
INFO:flit.install:Symlinking /home/user/Dokumente/git/flit/flit_core/flit_core -> /home/user/.local/lib/python3.9/site-packages/flit_core
Traceback (most recent call last):
  File "/home/user/Dokumente/git/flit/./bootstrap_dev.py", line 39, in <module>
    Installer(
  File "/home/user/Dokumente/git/flit/flit/install.py", line 413, in install
    self.install_directly()
  File "/home/user/Dokumente/git/flit/flit/install.py", line 331, in install_directly
    self.write_dist_info(dirs['purelib'])
  File "/home/user/Dokumente/git/flit/flit/install.py", line 357, in write_dist_info
    metadata = common.make_metadata(self.module, self.ini_info)
  File "flit_core/flit_core/common.py", line 388, in make_metadata
    return Metadata(md_dict)
  File "flit_core/flit_core/common.py", line 321, in __init__
    self.version = data.pop('version')
KeyError: 'version'

Metadata always contains the line "License: UNKNOWN" regardless of the project.license table.

if 'email' in person:
email = person['email']
if 'name' in person:
email = '{} <{}>'.format(person["name"], email)
Copy link

Choose a reason for hiding this comment

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

Users can include weird stuff in this field and break the formatting. Not sure if this is an issue.

{ name = "Jane\n\n\nDoe <a@b.c>", email="jane.doe@example.org" }

One could use formataddr.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I didn't know about that function. It somewhat garbles non-ASCII text, though:

In [10]: email.utils.formataddr(('Zoë', 'zoe@example.com>'))
Out[10]: '=?utf-8?q?Zo=C3=AB?= <zoe@example.com>>'

I don't know if that's what should happen. The original version metadata spec 1.0 (PEP 241) says the format is based on email headers, and presumably that's what email headers do. But that was almost 20 years ago, and it's possible that tools expect something a bit more Unicode native now. PEP 621 explicitly suggests "the format {name} <{email}>".

I'll open a discussion on the Python discourse about this.

Copy link
Member Author

Choose a reason for hiding this comment

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

@takluyver
Copy link
Member Author

Thanks for testing it - the issue with bootstrap_dev.py should be fixed now

@takluyver takluyver closed this Feb 28, 2021
@takluyver takluyver reopened this Feb 28, 2021
@takluyver
Copy link
Member Author

(wrong button)

@pyfisch
Copy link

pyfisch commented Mar 1, 2021

I tested the PR a bit more and found a few issues with the install subcommand:

  1. If the version is only specified under [project] the command flit install fails while flit build succeeds. (Message: Cannot package module without a version string. Please define a __version__ = "x.y.z" in your module.)
  2. If the pyproject.toml file doesn't contain a [build-system] table flit install does not emit an error but the package isn't installed either.
  3. If the installation doesn't fail for the two previous errors there is a "flit_core.config.ConfigError: TOML file missing [tool.flit] table." error.

This is the example project I used to test flit: pythagoras.zip

@uranusjr
Copy link
Member

uranusjr commented Mar 7, 2021

I can’t seem to make either build or install work. install says TOML file missing [tool.flit] table.

@pyfisch
Copy link

pyfisch commented Mar 7, 2021

@uranusjr Have you tried uninstalling flit before installing the version from this PR with pip uninstall flit flit_core?

@uranusjr
Copy link
Member

uranusjr commented Mar 7, 2021

I installed this branch into a fresh virtual environment. Maybe I messed up something, will check when I got more time.

@takluyver
Copy link
Member Author

@uranusjr if you installed from this branch in some obvious ways, you may have only installed flit (the command-line tools) and not flit_core. Running .../env/bin/python bootstrap_dev.py should set you up with both from the repo (& as editable installs).

@takluyver
Copy link
Member Author

There's a wrinkle in trying out flit install, which is actually a wrapper around pip install ., with a development branch. pip reads pyproject.toml, makes a build env, and installs a released version of flit_core from PyPI, which of course doesn't know about PEP 621. pyproject.toml files using PEP 621 should specify that their build-system requires = ["flit_core >=3.2,<4"].

If I use pip wheel . to make a local wheel of flit_core and then run PIP_FIND_LINKS=~/Code/flit/flit_core/ flit install, then it seems to work.

@takluyver
Copy link
Member Author

I'm thinking about where to put the importable module name if it differs from the distribution name. I'm currently thinking of something like this:

[project]
name = "pynsist"
#...

[tool.flit.module]
name = "nsist"

I.e. you would pip install pynsist but import nsist.

Does that seem reasonable? Is there a clearer or more useful way of writing this? Part of my thinking is that the [tool.flit.module] table could later gain fields to specify... something. More ways to control the importable module that gets installed. Maybe. 🤔

@uranusjr
Copy link
Member

Yeah that feels right by me. Is specifying metadata with [tool.flit] going to be deprecated or something after PEP 621 support is fully implemented and released? If not, something probably needs to be done if the user combine the various similar fields:

[tool.flit]
name = "x"
dist-name = "y"
module = { name = "z" }

@takluyver
Copy link
Member Author

My thinking is that people will use either the older style [tool.flit.metadata] table, or the new style [project] + [tool.flit.module], but won't be allowed to mix them (i.e. Flit will error if it sees a mixture).

Eventually I'd expect to remove support for [tool.flit.metadata], but not with any urgency. pyproject.toml files created by flit init currently specify that the project should be built with flit_core >=2,<4, so the ground is prepared for doing a breaking change to the configuration in v4.

@takluyver
Copy link
Member Author

I'm thinking of releasing this as an experimental feature, and encouraging people to specify the build-system requirement as flit_core >=3.2,<3.3 (for now) if they use it, so it's possible to make some changes to how it works if needed.

@flying-sheep
Copy link
Contributor

Ah, and since it’s experimental, you don’t yet use it in the readme or docs, I assume.

@takluyver
Copy link
Member Author

Yup precisely. It's mentioned in the release notes, and I'd encourage early adopters to try using it, and then hopefully it can be documented for version 3.4.

One question I'd like to resolve is what it should do if there are extra fields in [project] - should it assume that extra metadata is probably not essential and carry on as if it wasn't there? Or error out if there's anything it doesn't recognise? Currently it warns but carries on. We're discussing this on the Python discourse.

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

Successfully merging this pull request may close these issues.

PEP 621 implementation (package metadata in pyproject.toml)
4 participants