ENH: add support for editable wheels #87

FFY00 · 2022-06-22T17:41:40Z

This is just an initial approach, that is known to not work on all
use-cases. It simply uses .pth files to inject search locations for the
default import finder, for this reason generated files that should be
installed inside modules will not be available.

Signed-off-by: Filipe Laíns [email protected]

FFY00 · 2022-11-25T08:07:51Z

This new implementation uses import hooks, which is what I wanted to do initially. Since the complexity required to make the old approach work turned out to be a bit big, it makes more sense to do it like this from the start.

The idea is that we built on top of the Traversable protocol (EditablePath) and only trigger a rebuild when the data is accessed, this will delay rebuilds until the very last moment and prevent unnecessary rebuilds by just importing the module.

dnicolodi · 2022-11-25T08:27:14Z

This is an interesting approach. Although I wonder how ergonomic it is. The main use of editable installations is during development. During development with compiled languages (which I think is the main target of meson-python) it is not unusual to do a recompilation just to check that the written code at least compiles. How would this work with approach? Also, does this still uses ephemeral build directories as the regular builds or does it default to a build directory inside the project folder? An example usage scenario would be great to have. Thank you!

rgommers · 2022-11-25T09:45:27Z

During development with compiled languages (which I think is the main target of meson-python)

I'd say it is "Python projects containing compiled languages" - typically the majority of contributors (at least for large projects like NumPy, SciPy, Pandas) touch docs, tests, and pure Python code only, or at least way more often than native code.

I agree that when you're touching C/C++/Cython/Fortran, editable builds are less relevant. But I've had regular requests from collaborators for editable installs, in particular from those who work with IDEs.

rgommers · 2022-11-25T09:49:17Z

The way we've set it up for SciPy now to work with Meson is that running the test suite always first rebuilds (necessary to ensure the out-of-place install is not out of date). This takes a fraction of a second, so avoiding rebuilds on imports isn't essential I'd say. (not sure it matters regarding code complexity here).

rgommers · 2022-11-25T09:56:33Z

The idea is that we built on top of the Traversable protocol (EditablePath) and only trigger a rebuild when the data is accessed, this will delay rebuilds until the very last moment and prevent unnecessary rebuilds by just importing the module.

The Python stdlib docs are extremely sparse as usual. I have to admit I don't immediately grasp how this all works, a sketch of what actually happens with this implementation when one runs pip install -e . and then import pkg would be useful.

dnicolodi · 2022-11-25T10:13:13Z

I agree that when you're touching C/C++/Cython/Fortran, editable builds are less relevant.

I don't think that's the case. I do a lot of cross-project development where I need to test the changes in one project within another one. In this case editable installations are useful.

lithomas1 · 2022-11-28T16:06:05Z

Wow, this is pretty cool. Thanks for putting up the PR @FFY00.
I just have a couple quick clarifying questions.

For this to work, would every project would need to inject this hook(MesonpyFinder) into sys.meta_path in their init.py?

lithomas1 · 2022-11-28T16:06:58Z

mesonpy/_editable.py

+
+def install(path: str) -> None:
+    editable = Editable(os.path.abspath(path))
+    sys.meta_path.append(MesonpyFinder(editable))


Would a prepend be more appropriate here?

Doesn't matter much, but my reasoning for simply appending was to not slow down any of the other imports. But this is now outdated given our new approach.

FFY00 · 2022-11-28T19:50:38Z

After discussing with @rgommers, we thought it would be better to simplify the initial approach. Instead of creating our own loader and reader, we will just intercept the import call with a custom finder, which won't change any of the import machinery and will simply trigger a rebuild + install to a dummy directory with we will make sure we add to sys.path. This means that the module would still be imported via the normal finder and loader. I think this will be easier to understand after you see the code.

FFY00 · 2022-11-28T19:53:51Z

For this to work, would every project would need to inject this hook(MesonpyFinder) into sys.meta_path in their init.py?

Nop! Our editable wheel will install a .pth file that will do that. This code is still missing from the PR.

Also, the custom finder will need to be in sys.meta_path before the module is imported, as its goal is to hook into the import process, and the module __init__.py gets run on import, so that wouldn't work 😛

mesonpy/_editable.py

henryiii · 2022-11-30T03:33:19Z

mesonpy/__init__.py

@@ -192,11 +188,15 @@ def _has_extension_modules(self) -> bool:
        # Assume that all code installed in {platlib} is Python ABI dependent.
        return bool(self._wheel_files['platlib'])

+    @property
+    def normalized_name(self) -> str:
+        return self._project.name.replace('-', '_')


Not related to editable installs: shouldn't this also map "." to "_"?

Good catch! But not really here, though I should definitely be doing that in the module name.

FFY00 · 2022-11-30T03:47:50Z

Alright, I haven't tested it yet, but it's getting pretty late, so I should go. The code should now be complete, but I expect stuff to be broken -- as I said, I didn't do any testing.

dnicolodi

Just a few superficial comments.

mesonpy/__init__.py

dnicolodi · 2022-11-30T10:10:13Z

mesonpy/__init__.py

+
+        project_path = self._source_dir / '.mesonpy-editable'
+        top_level_modules = self._project.top_level_modules
+        rebuild_commands = [list(cmd) for cmd in self._project.build_commands(project_path)]


Why the list comprehension?

To convert to the correct type.

Correct type for what? This makes tuples into lists, but it does not seem like lists are required. Are they? If they are, can't we just return the correct type?

mesonpy/__init__.py

lithomas1

I left some comments, addressing typos I saw when I pulled this. Hopefully, this helps you out a little.

Other then the suggestions, you'll need to add mesonpy/_editable to the meson.build file.

mesonpy/__init__.py

FFY00 · 2022-11-30T19:14:36Z

Alright, this should now be finally working 🎉

The only thing is that it currently does not detect top-level native modules.

@lithomas1 want to give it a try?

lithomas1 · 2022-11-30T21:58:19Z

Alright, this should now be finally working 🎉

The only thing is that currently does not detect top-level native modules.

@lithomas1 want to give it a try?

Nice this is pretty awesome 🚀 (It is especially cool that you can just edit .pyx files and have the changes sync up too).

The only issue I have is slow rebuilds.
I attached time.time() calls to measure the speed of rebuilds and currently its' taking ~3-4 seconds for a regular rebuild, and ~34 seconds(!!) when I modified a .pyx file.

It's not a deal-breaker for me, but its probably something to think about in the future.

FFY00 · 2022-11-30T22:17:50Z

How does that compare to just running ninja directly? It shouldn't be taking much longer than that, and this is something we can't really do much about.
Using Traversables we should be able to avoid the extra install step, but that shouldn't take that long anyway. We could also try to optimize the import hook code, but I feel that will be insignificant compared to the build step.

Perhaps we should have a way to disable swallowing the Meson output, so that developers can see the log when Python starts up?

Signed-off-by: Filipe Laíns <[email protected]>

This was leftover from when we ran auditwheel. Signed-off-by: Filipe Laíns <[email protected]>

Signed-off-by: Filipe Laíns <[email protected]>

pytest has its own tmpdir management and cleanup, making is easier to inspect the environments when the tests fail. Signed-off-by: Filipe Laíns <[email protected]>

Signed-off-by: Filipe Laíns <[email protected]>

Dirty workaround for cases where Meson does not install to the expected paths, like in macOS. Signed-off-by: Filipe Laíns <[email protected]>

Signed-off-by: Filipe Laíns <[email protected]>

lithomas1 · 2022-12-23T15:47:28Z

docs/index.rst

+
+.. code-block::
+
+   python -m pip install -e . --config-settings editable-verbose=


Do we error when this is passed but we are not doing an editable install?

Yep.

meson-python/mesonpy/__init__.py

Lines 1119 to 1128 in 89af86c

for key in config_settings:

known_keys = ('builddir', 'editable-verbose', *meson_args_cli_keys)

if key not in known_keys:

import difflib

matches = difflib.get_close_matches(key, known_keys, n=3)

if len(matches):

alternatives = ' or '.join(f'"{match}"' for match in matches)

raise ConfigError(f'Unknown configuration entry "{key}". Did you mean {alternatives}?')

else:

raise ConfigError(f'Unknown configuration entry "{key}"')

rgommers

The docs look quite good to me now, just a few minor suggestions to clarify things.

docs/index.rst

lithomas1

LGTM

Signed-off-by: Filipe Laíns <[email protected]>

FFY00 · 2022-12-28T16:24:09Z

I somehow missed the notification for Ralf's review, so I only noticed yesterday 🤣

This comment was marked as outdated.

Sign in to view

FFY00 force-pushed the editable-wheels branch 2 times, most recently from 4a2b9df to 034d9d1 Compare November 25, 2022 08:03

FFY00 force-pushed the editable-wheels branch from 034d9d1 to cee00c5 Compare November 25, 2022 08:10

rgommers added the enhancement New feature or request label Nov 25, 2022

lithomas1 mentioned this pull request Nov 27, 2022

TRACKER: Getting Started with Meson/Bug Reports pandas-dev/pandas#49683

Open

4 tasks

lithomas1 reviewed Nov 28, 2022

View reviewed changes

FFY00 force-pushed the editable-wheels branch from cee00c5 to 64625c2 Compare November 28, 2022 20:03

FFY00 mentioned this pull request Nov 30, 2022

ENH: Hacking Python's import system to find the installed package even in the source directory #239

Closed

lithomas1 reviewed Nov 30, 2022

View reviewed changes

mesonpy/_editable.py Show resolved Hide resolved

FFY00 force-pushed the editable-wheels branch from 64625c2 to 400c62e Compare November 30, 2022 02:42

henryiii reviewed Nov 30, 2022

View reviewed changes

FFY00 force-pushed the editable-wheels branch from 400c62e to 5ee52b1 Compare November 30, 2022 03:42

dnicolodi reviewed Nov 30, 2022

View reviewed changes

lithomas1 reviewed Nov 30, 2022

View reviewed changes

mesonpy/__init__.py Outdated Show resolved Hide resolved

mesonpy/__init__.py Outdated Show resolved Hide resolved

mesonpy/__init__.py Outdated Show resolved Hide resolved

mesonpy/__init__.py Outdated Show resolved Hide resolved

mesonpy/__init__.py Outdated Show resolved Hide resolved

FFY00 force-pushed the editable-wheels branch 2 times, most recently from 53512e0 to bd85f57 Compare November 30, 2022 19:09

FFY00 added 15 commits December 23, 2022 13:16

ENH: add support for editable installs

7c764be

Signed-off-by: Filipe Laíns <[email protected]>

MAINT: simplify Project.wheel

e99b3d0

This was leftover from when we ran auditwheel. Signed-off-by: Filipe Laíns <[email protected]>

MAINT: use pathlib.Path.as_posix in _WheelBuilder._install_path

72e88ea

Signed-off-by: Filipe Laíns <[email protected]>

BUG: fix rebuild loop when the build tries to import the editable module

569a707

Signed-off-by: Filipe Laíns <[email protected]>

ENH: add editable-verbose config setting

c22ec4d

Signed-off-by: Filipe Laíns <[email protected]>

TST: add VEnv.python and VEnv.pip helpers

ebc70da

Signed-off-by: Filipe Laíns <[email protected]>

TST: use pytest's tmp_path_factory fixture in venv creation

f64dca1

pytest has its own tmpdir management and cleanup, making is easier to inspect the environments when the tests fail. Signed-off-by: Filipe Laíns <[email protected]>

TST: add test for editable wheels

0b4ac54

Signed-off-by: Filipe Laíns <[email protected]>

TST: add test for top_level_modules

72c8701

Signed-off-by: Filipe Laíns <[email protected]>

MAINT: use sysconfig in _debian_python on Python >=3.10

3416d0b

Signed-off-by: Filipe Laíns <[email protected]>

MAINT: move top_level_modules to _WheelBuilder

1833569

Signed-off-by: Filipe Laíns <[email protected]>

TST: ignore couldnt-parse warning in coverage

d0dc5d5

Signed-off-by: Filipe Laíns <[email protected]>

BUG: fix editable import path calculation when Meson does not match

7d7ff76

Dirty workaround for cases where Meson does not install to the expected paths, like in macOS. Signed-off-by: Filipe Laíns <[email protected]>

MAINT: split path introspection into _introspection module

2305efb

Signed-off-by: Filipe Laíns <[email protected]>

BUG: fix deb_system sysconfing scheme addition version

920c4bd

Signed-off-by: Filipe Laíns <[email protected]>

FFY00 force-pushed the editable-wheels branch from 376ed35 to 89af86c Compare December 23, 2022 13:16

lithomas1 reviewed Dec 23, 2022

View reviewed changes

rgommers approved these changes Dec 24, 2022

View reviewed changes

docs/index.rst Outdated Show resolved Hide resolved

docs/index.rst Outdated Show resolved Hide resolved

docs/index.rst Outdated Show resolved Hide resolved

docs/index.rst Outdated Show resolved Hide resolved

lithomas1 approved these changes Dec 27, 2022

View reviewed changes

FFY00 added 2 commits December 28, 2022 16:19

DOC: add simple editable installs documentation

c10fc01

Signed-off-by: Filipe Laíns <[email protected]>

ENH: add error if we can't locate the project in the editable hook

d0011ec

Signed-off-by: Filipe Laíns <[email protected]>

FFY00 force-pushed the editable-wheels branch from 89af86c to d0011ec Compare December 28, 2022 16:19

FFY00 merged commit 0f0e87a into main Dec 28, 2022

FFY00 deleted the editable-wheels branch December 28, 2022 16:30

FFY00 mentioned this pull request Dec 28, 2022

Add support for editable wheels #47

Closed

rgommers added this to the v0.13.0 milestone Dec 28, 2022

rgommers mentioned this pull request Dec 28, 2022

Tracking issue for further build system improvements scipy/scipy#16293

Open

16 tasks

lithomas1 mentioned this pull request Jan 29, 2023

BUG: Deleting a file from an editable install #282

Closed

conradtchan mentioned this pull request Jan 31, 2023

Editable installs conradtchan/starfit#210

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: add support for editable wheels #87

ENH: add support for editable wheels #87

FFY00 commented Jun 22, 2022 •

edited

Loading

This comment was marked as outdated.

FFY00 commented Nov 25, 2022

dnicolodi commented Nov 25, 2022

rgommers commented Nov 25, 2022

rgommers commented Nov 25, 2022

rgommers commented Nov 25, 2022

dnicolodi commented Nov 25, 2022

lithomas1 commented Nov 28, 2022

lithomas1 Nov 28, 2022

FFY00 Nov 28, 2022

FFY00 commented Nov 28, 2022

FFY00 commented Nov 28, 2022

henryiii Nov 30, 2022

FFY00 Nov 30, 2022

FFY00 commented Nov 30, 2022

dnicolodi left a comment

dnicolodi Nov 30, 2022

FFY00 Nov 30, 2022

dnicolodi Dec 1, 2022

lithomas1 left a comment

FFY00 commented Nov 30, 2022 •

edited

Loading

lithomas1 commented Nov 30, 2022 •

edited

Loading

FFY00 commented Nov 30, 2022

lithomas1 Dec 23, 2022

FFY00 Dec 23, 2022

rgommers left a comment

lithomas1 left a comment

FFY00 commented Dec 28, 2022


		.. code-block::

		python -m pip install -e . --config-settings editable-verbose=

	for key in config_settings:
	known_keys = ('builddir', 'editable-verbose', *meson_args_cli_keys)
	if key not in known_keys:
	import difflib
	matches = difflib.get_close_matches(key, known_keys, n=3)
	if len(matches):
	alternatives = ' or '.join(f'"{match}"' for match in matches)
	raise ConfigError(f'Unknown configuration entry "{key}". Did you mean {alternatives}?')
	else:
	raise ConfigError(f'Unknown configuration entry "{key}"')

ENH: add support for editable wheels #87

ENH: add support for editable wheels #87

Conversation

FFY00 commented Jun 22, 2022 • edited Loading

This comment was marked as outdated.

FFY00 commented Nov 25, 2022

dnicolodi commented Nov 25, 2022

rgommers commented Nov 25, 2022

rgommers commented Nov 25, 2022

rgommers commented Nov 25, 2022

dnicolodi commented Nov 25, 2022

lithomas1 commented Nov 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FFY00 commented Nov 28, 2022

FFY00 commented Nov 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FFY00 commented Nov 30, 2022

dnicolodi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lithomas1 left a comment

Choose a reason for hiding this comment

FFY00 commented Nov 30, 2022 • edited Loading

lithomas1 commented Nov 30, 2022 • edited Loading

FFY00 commented Nov 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgommers left a comment

Choose a reason for hiding this comment

lithomas1 left a comment

Choose a reason for hiding this comment

FFY00 commented Dec 28, 2022

FFY00 commented Jun 22, 2022 •

edited

Loading

FFY00 commented Nov 30, 2022 •

edited

Loading

lithomas1 commented Nov 30, 2022 •

edited

Loading