kuadrat / data-slicer

Multidimensional data visualization tools
MIT License
9 stars 4 forks source link

Can't get attribute `ds_cmap` #3

Closed kuadrat closed 3 years ago

kuadrat commented 3 years ago

Issue originally formulated by @sabinomaggi

Hi Kevin, I have followed the installation instructions in https://data-slicer.readthedocs.io/en/latest/installation.html and everything seems OK.

(base) $ conda create --name dataslicer python==3.7.5
(base) $ conda activate dataslicer
(dataslicer) $ pip install --upgrade pip
[...]
Successfully installed pip-21.0.1

(dataslicer) $ pip install data_slicer
Collecting data_slicer
[...]
Installing collected packages: ipython-genutils, wcwidth, traitlets, six, [...]
Successfully installed [...] data-slicer-0.0.9 [...]

However, when I try to test the installation by running pit I get this error message

(dataslicer) $ pit 
===== 2021.02.08 14:02:05 =====
Traceback (most recent call last):
  File "/Users/maggi/anaconda3/envs/dataslicer/bin/pit", line 5, in <module>
    from data_slicer.pit import start_main_window
  File "/Users/maggi/anaconda3/envs/dataslicer/lib/python3.7/site-packages/data_slicer/pit.py", line 75, in <module>
    cmaps = pickle.load(f)
AttributeError: Can't get attribute 'ds_cmap' on <module '__main__' from '/Users/maggi/anaconda3/envs/dataslicer/bin/pit'>

I have also repeated this precedure using the latest available version of Python and the error is exactly the same.

I have not been able to find any info about package compatibility in the documentation (and also in the paper under review), and therefore it is possible that data-dlicer runs only under Linux. However, if this is true, I think you should explicitly state this fact in the documentation, as well as in the paper (but this last point is clearly part of the review). If, on the other hand, data-dlicer is supposed to run also under macOS, could you please help to solve this issue?

kuadrat commented 3 years ago

Hi @sabinomaggi and thanks again for the detailed feedback!

Indeed, compatibility information is missing and should be there. I will add it to the readme and documentation. In principle, the software should run on Linux, Windows and macOS.

I actually suspect that the error here arises due to a last minute feature that I have implemented shortly before the submission to JOSS. Namely a form of colormap caching, intended to decrease startup time.

To be more precise, in an older version, all the colormap objects would be created on startup. I noticed that this took up time in the order of seconds, which is not ideal, especially since usually not many colormaps will be used during a session. So I thought instead of recreating them on each startup, I could just load previously pickled objects - which eventually seemed to work but only after a lot of trail and error. There's likely something unstable with the way it's implemented now, still.

I'll see if I can fix it, but suspect that I will likely just have to remove the feature and live with longer start up times for now.

sabinomaggi commented 3 years ago

Hi @kuadrat , thanks for the quick reply. If you want I can clone the repo and go back to a previous commit to check if your assumption is true (at least on macOS).

kuadrat commented 3 years ago

So my first hunch is probably correct. This is not a compatibility issue, but a proper bug. I didn't notice it, because when testing, I would start pit using python -m data_slicer.pit instead of the more convenient pit entry point. Maybe I can find out why the unpickling works in one case but not in the other.

Can you check if python -m data_slicer.pit works for you?

kuadrat commented 3 years ago

Hi @kuadrat , thanks for the quick reply. If you want I can clone the repo and go back to a previous commit to check if your assumption is true (at least on macOS).

That would be a pathway we could follow if python -m data_slicer.pit doesn't work. The commit in question would be be5d0991f3b20080b0b66548a36b6b91d536a79a. Most commits after that pertain to clean up and paper writing, so we shouldn't miss any essential features by working with that version.

kuadrat commented 3 years ago

Thanks to your input I was able to think of a much more straightforward way to handle caching that should not lead to the issue described. If you could test again with the newest version (0.0.10), I'd be happy to hear the results.

sabinomaggi commented 3 years ago

@kuadrat python -m data_slicer.pit works and all the elements of the interface respond to my mouse clicks (now I must discover how to use my own data).

sabinomaggi commented 3 years ago

Thanks to your input I was able to think of a much more straightforward way to handle caching that should not lead to the issue described. If you could test again with the newest version (0.0.10), I'd be happy to hear the results.

Of course yes, takes nothing...

$ pip install data_slicer --upgrade
[...]
Installing collected packages: data-slicer
  Attempting uninstall: data-slicer
    Found existing installation: data-slicer 0.0.9
    Uninstalling data-slicer-0.0.9:
      Successfully uninstalled data-slicer-0.0.9
Successfully installed data-slicer-0.0.10

You are right, now pit works perfectly and takes nothing to start.

sabinomaggi commented 3 years ago

I think this issue can be closed, now.