Resolves #660

When building this package using pip install --use-pep517 --no-binary :all: or in dev pip install -e . --use-pep517 we see a failure in setup.py:

  [...]
  AttributeError: 'dict' object has no attribute '__LIGHTFM_SETUP__'

__builtins__ is an implementation detail and:

The value of __builtins__ is normally either this module or the value of this module’s __dict__ attribute

When using pip with --use-pep517 it seems to be a dict, and otherwise seems to be the builtins object.

Rather than switch to importing the builtins module I've taken the opportunity to rework the package version code to follow approach (3) detailed in https://packaging.python.org/en/latest/guides/single-sourcing-package-version/

This feels a little more forgiving than the current approach in that it avoids loading the actual package itself in setup.py, skipping the workaround for the fact LightFM relies on a native extension.

Questions

does the existing approach of importing __version__ from LightFM seem better for some reason? Happy to shrink this PR down to just a replacement of __builtins__ with the builtins module
is there any other metadata we'd want to pull out of setup.py and into a more general about.py file? In the example the "single-sourcing-package-version" page links to for (3) their file contains lots of metadta: copyright, license, author, etc.
testing: I'm not really sure how to go about avoiding this issue in the future. It seems like anything at the unit level is likely to be meaningless, but running pip install a few times with different flags feels very heavyweight. Is there a useful spot somewhere in between that I'm missing?

Sep 16 '22 08:09 daniel-ferguson

@maciejkula @SimonCW What is the best way to get this merged?

Sep 19 '22 15:09 TNonet

@daniel-ferguson This PR seems to be failing the CI checks. Do you need assistance with this?

@maciejkula @SimonCW Is there a process or active maintainer to assist with merging this PR?

Oct 10 '22 14:10 TNonet

@TNonet I've fixed the previous lint error but besides that I think there's a bit of bitrot at play, unrelated to changes in this PR, that should probably be resolved separately.

In the circle-ci lint step:

Traceback (most recent call last):
  File "/home/circleci/project/venv/bin/black", line 8, in <module>
    sys.exit(patched_main())
  File "src/black/__init__.py", line 1423, in patched_main
  File "src/black/__init__.py", line 1409, in patch_click
ImportError: cannot import name '_unicodefun' from 'click' (/home/circleci/project/venv/lib/python3.8/site-packages/click/__init__.py)

In the appveyor build it looks like there are native code compilation errors, e.g.

...
lightfm/_lightfm_fast_no_openmp.c(24538): error C2105: '++' needs l-value
lightfm/_lightfm_fast_no_openmp.c(24540): error C2105: '--' needs l-value
lightfm/_lightfm_fast_no_openmp.c(24849): error C2105: '++' needs l-value
lightfm/_lightfm_fast_no_openmp.c(24851): error C2105: '--' needs l-value
lightfm/_lightfm_fast_no_openmp.c(25099): error C2105: '++' needs l-value
lightfm/_lightfm_fast_no_openmp.c(25101): error C2105: '--' needs l-value
...

Resolving these would definitely need more of a time investment than I'm capable of at the moment.

Oct 18 '22 20:10 daniel-ferguson

Closing as this was resolved in an updated version of this PR: https://github.com/lyst/lightfm/pull/685

Jul 26 '23 09:07 daniel-ferguson

Fix build for pip with --use-pep517

Questions