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

Update pyproject.toml #67

Closed
wants to merge 1 commit into from
Closed

Conversation

bbradbury
Copy link

I think this is a safe change - lttbc has been updated to 0.2.1

I think this is a safe change - lttbc has been updated to 0.2.1
@jvdd
Copy link
Member

jvdd commented May 24, 2022

Hi,

Thanks for your contribution! 😄

I would love to merge it into main, but there seems to be a problem with lttbc 0.2.1 (see #42). The compiled files use a numpy version that is rather recent, resulting in warnings for the current numpy versions plotly-resampler supports.

I'm interested to hear your thoughts about this!

Cheers,
Jeroen

PS: @jonasvdd is currently looking into implementing & compiling a more optimized version of lttbc 🚀

@zxweed
Copy link

zxweed commented May 24, 2022

Another evidence of lttbc's problematic behavior is that a strange error occurs when you try to deploy the configuration to binder:

Collecting lttbc==0.1.9
  Downloading lttbc-0.1.9.tar.gz (4.4 kB)
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'error'
  ERROR: Command errored out with exit status 1:
   command: /srv/conda/envs/notebook/bin/python3.8 -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-uhjh_3bk/lttbc_640214e8567d4d14a66351971e1a2f74/setup.py'"'"'; __file__='"'"'/tmp/pip-install-uhjh_3bk/lttbc_640214e8567d4d14a66351971e1a2f74/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-faa8ruak
       cwd: /tmp/pip-install-uhjh_3bk/lttbc_640214e8567d4d14a66351971e1a2f74/
  Complete output (5 lines):
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File "/tmp/pip-install-uhjh_3bk/lttbc_640214e8567d4d14a66351971e1a2f74/setup.py", line 7, in <module>
      import numpy
  ModuleNotFoundError: No module named 'numpy'
  ----------------------------------------
WARNING: Discarding https://files.pythonhosted.org/packages/b1/56/92bd8ddf5ffa4ba46b5d818c5243e4912443d3aa8bf72ec5a190828ea0d9/lttbc-0.1.9.tar.gz#sha256=35b375c4fd26c358f33f9a8f579024e5596cde0ed90e33f8be105af26fab8951 (from https://pypi.org/simple/lttbc/) (requires-python:>=3.5). Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
ERROR: Could not find a version that satisfies the requirement lttbc==0.1.9 (from versions: 0.1.9, 0.2.0, 0.2.1)
ERROR: No matching distribution found for lttbc==0.1.9

So I very much appreciate the attempt to replace this package with the potentially more efficient fast-lttb.

@jonasvdd
Copy link
Member

jonasvdd commented May 24, 2022

Hi,

Some problems with lttbc already popped up in #65, #32 and #42.

As @jvdd already mentioned, I'm currently looking into implementing the core components of EfficientLTTB in c, see this repo. However, I cannot estimate how long this will take (have some other deadlines / trips coming up the next weeks), as I do not have much experience in compiling platform-agnostic python bindings for c code (which seems one of the major problems now with lttbc). So if you have any experience regarding that, feel free to help me out!

@zxweed, from your error it seems that lttbc requires numpy, so maybe you can circumvent this by first (explicitly) installing numpy in your python environment after which you try to add lttbc? (I'm also intrigued by the lttbc-version 0.1.9, as our pyproject.toml now states lttbc=0.2.0, which is a strict version requirement)

fyi: a colleague of mine @mcourteaux is also looking into a really optimized version of these algorithms, leveraging the SIMD vector instruction set, see this repo

@zxweed
Copy link

zxweed commented May 24, 2022

I specified numpy 1.21 in the requirements and it successfully installed above in the log. Versions 0.1.9 of lttbc were left from trying to find one that worked out of the three - no luck.

@jonasvdd
Copy link
Member

@zxweed, If you look at the original lttbc repo, the requirements.txt lists numpy>=1.16. So I would guess that the lttbc python bindings were generated with the numpy 1.16 python-C version. And this might cause the incompatibility with later /prior numpy versions.

@bbradbury
Copy link
Author

Thanks @jonasvdd @zxweed! Understood on the wackiness with the new version of lttbc. If I get cycles I will have a look at the EfficientLTTB code and I will also socialize it with my team at work in case anyone here can help.

@bbradbury
Copy link
Author

bbradbury commented May 24, 2022

Also, for what it's worth, here's what I did to make lttbc install in a conda environment on an m1 mac, since there are not binaries published for osx/arm:

#In a 'conda activate' environment....

#Optional - speed up by installing libmamba solver
#blog: https://www.anaconda.com/blog/a-faster-conda-for-a-growing-community
conda install -n base conda-libmamba-solver


pip install lttbc==0.2.0
conda config --set pip_interop_enabled true
#Now normal conda commands will work...

#If you did the optional solver, reference it when you install the environment:
conda env update --experimental-solver=libmamba --file environment.yml --prune

@bbradbury bbradbury closed this May 26, 2022
@jonasvdd
Copy link
Member

Hi @bbradbury

In this PR #84, I am to add my own LTTB implementation to plotly-resampler with the aim of (1) reducing install errors and (2) adding optimized implementations of this algorithm.

Feel free to take a look / give some thoughts if you have time.
Additionally, plotly-resamplers' license was changed to MIT 🥳

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.

4 participants