BIOP / abba_python

ABBA, controlled from python
BSD 3-Clause "New" or "Revised" License
6 stars 3 forks source link

Fixed import error #17

Closed GuillaumeLeGoc closed 2 months ago

GuillaumeLeGoc commented 3 months ago

See here : https://github.com/BIOP/abba_python/pull/15

GuillaumeLeGoc commented 3 months ago

For information, while I'm here : The fork is here : https://github.com/GuillaumeLeGoc/abba_python In the main branch, there are others commits (in the middle of reverting others), including :

Unfortunately, I was not able to use ABBA 0.8.2, that includes the fix for this annoying bug. With brainglobe atlases, the GUI is launched but the coordinates do not make any sense (jumping from -100 to -1000 from one slice to another) and atlas images disappearing and repapering seemingly randomly, both for allen_mouse_10um and allen_cord_20um, and both with the old bg-atlasapi and the new brainglobe-atlasapi. Regular CCFv3 mouse brain works as before.

Anyway, just to keep you updated, cheers !

NicoKiaru commented 3 months ago

Thanks for the feedback. The jumping is weird, and I don't really see how it can be related to v0.8.2. Thanks for the explanation, that'll be helpful to understand what is wrong.

NicoKiaru commented 2 months ago

Hi @GuillaumeLeGoc ,

I'll make a new release soon, if you want to merge your other commits please make another PR.

I'll focus on updating the java version to 0.9.1 and maybe other python packages for now, and will wait before the release.

NicoKiaru commented 2 months ago

The import error still shows up, it's a nightmare. I'll restore the try / catch this

GuillaumeLeGoc commented 2 months ago

Aw. This is weird. I installed it from my fork a couple of days ago for someone in my lab and it worked with the spinal cord brainglobe atlas... From the downloaded repo :

conda create -c conda-forge -n abba-env python=3.10 openjdk=11 maven pyimagej
conda activate abba-env
pip install .

Using Windows 10

NicoKiaru commented 2 months ago

Maybe it's pycharm that messes things up... Maybe that's fine indeed.

FYI I never really explained why the import are delayed and not done just once and for all - and that's because abba-map, abba-ontology and the other ones are dependent on Java being started. So ABBA has to be instantiated before the rest is imported.

Also I want to support these two cases:

(+And we can't start several Java with JPype, the engine behing Java being used in python)

NicoKiaru commented 4 weeks ago

@GuillaumeLeGoc I'm still stuck with this )(/&ç)(/&ç%&# problem.

I tried this:

https://github.com/BIOP/abba_python/blob/4b8ea4d6731a8f7c8e4d7be18540ddcf3c3466c0/src/abba_python/abba.py#L121-L133

And this does not even work when I run after abba python is intalled from the installer I made with conda constructor. (it works in pycharm, it works with a script).

Sorry to write that, but I'm really impressed by the shittiness of python imports. I never have to worry about a package import in Java for tens of years and I suddenly realized that what I took for granted can be completely messed up.

Unrelated, but I plan to make a release in the coming month. I'd be nice to get these axis problem fixed. If you have time, you can look at the latest release: https://pypi.org/project/abba-python/0.9.1.dev1/ and if we can't get this solved by message, maybe we could try a small zoom debug session ?

GuillaumeLeGoc commented 4 weeks ago

Hey Nico, nice to hear from you and good job for the refactoring. I'm not at the office right now so I won't be able to test this extensively today but next week we can definitely have a chat if we (you) don't fix this till then. FYI I'm running Linux at the moment. Installing abba-python==0.9.1.dev1 in a fresh conda environment and trying, from IPython directly in a terminal :

from abba_python import Abba
aligner = Abba("allen_mouse_10um")

results in...

ModuleNotFoundError                       Traceback (most recent call last)
Cell In[6], line 1
----> 1 aligner = Abba("allen_mouse_10um")

File ~/miniforge3/envs/abba-dev/lib/python3.10/site-packages/abba_python/abba.py:250, in Abba.__init__(self, atlas_name, ij, slicing_mode, headless, print_config, log_level)
    248 bg_atlas = BrainGlobeAtlas(atlas_name)
    249 # initialized
--> 250 from abba_atlas import AbbaAtlas
    251 atlas = AbbaAtlas(bg_atlas, ij)
    252 atlas.initialize(None, None)

ModuleNotFoundError: No module named 'abba_atlas'

I don't see where the get() method is used so your try/catch are not even reached I think.

I'm far from an expert and been facing the same kind of problems but what I understood is that if you have a bunch of python modules in a directory with the __init__.py file, it makes it a package. Then the modules are part of that package and it seems you can't just from module import something. You can either use relative import (from .module import something) or use the whole package (from package.module import something) .

I tried both on line 250 and it worked (then I got a ModuleNotFoundError: No module named 'abba_map' abba_atlas.py:6).

NicoKiaru commented 4 weeks ago

Thanks, I think I found the issue, the import that fails wasn't directly this one but a 'nested' one. Sometimes printing the error message helps!