python3Packages.tsfresh: 0.20.3 -> 0.21.0#416446
Conversation
|
Re-running CI due to a odd failure fixed in #416448 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/5586 |
jherland
left a comment
There was a problem hiding this comment.
LGTM with one minor nit.
There was a problem hiding this comment.
I cannot see setuptools listed in the declared dependencies for this package, so I rather suspect that it's only needed as a tool while building the package. In that case, I think it's more idiomatic to pass it in as
build-system = [ setuptools ];There was a problem hiding this comment.
Hmm actually I can't import the package unless setuptools is added to the python environment:
> nix-shell -p 'python3.withPackages (p: [p.tsfresh])' -I nixpkgs=. --run python3
Python 3.13.4 (main, Jun 3 2025, 15:34:24) [GCC 14.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tsfresh
Traceback (most recent call last):
File "<python-input-0>", line 1, in <module>
import tsfresh
File "/nix/store/lf4mlsp1k3iahmmclc7k29cxlrbx4fgv-python3-3.13.4-env/lib/python3.13/site-packages/tsfresh/__init__.py", line 12, in <module>
import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'
>>>
> nix-shell -p 'python3.withPackages (p: [p.tsfresh p.setuptools])' -I nixpkgs=. --run python3
Python 3.13.4 (main, Jun 3 2025, 15:34:24) [GCC 14.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tsfresh
>>>
It looks like it depends on it after all, though that is a bit odd
There was a problem hiding this comment.
If it indeed imports pkg_resources then it should be in dependencies, that's right. I think our pythonImportsCheckHook fails to catch this, because setuptools is present in PYTHONPATH during build and gone afterwards.
Should we have a separate test that imports packages after the build instead of or in addition to pythonImportsCheckHook?..
I'll move setuptools back into dependencies.
There was a problem hiding this comment.
Yeah, with what you describe, setuptools should indeed be in dependencies, and actually the upstream project is in error for not including setuptools in its declared dependencies (FawltyDeps would have caught this, dammit... 😅).
I believe this specific case is actually quite common, as setuptools (which provides the setuptools and pkg_resources import names) used to be auto-installed into any new virtualenvs (until Python 3.12), so a missing dependency declaration would typically go unnoticed.
https://github.com/blue-yonder/tsfresh/blob/refs/tags/v0.21.0/CHANGES.rst * disable tests on Python verions where pandas-daatareader doesn't work * add dependency on pywavelets that fixes incompatibility with the current version of scipy
|
Successfully created backport PR for |
https://github.com/blue-yonder/tsfresh/blob/refs/tags/v0.21.0/CHANGES.rst
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.