nucleic / enaml

Declarative User Interfaces for Python
http://enaml.readthedocs.io/en/latest/
Other
1.52k stars 130 forks source link

qt: fix loading dock images #503

Closed MatthieuDartiailh closed 1 year ago

MatthieuDartiailh commented 1 year ago

We test directly the loading since we cannot test D&D on the dock area

This is extremely annoying and I will do my best to release a patch version ASAP

MatthieuDartiailh commented 1 year ago

ping @bburan since you worked on the previous patch

codecov-commenter commented 1 year ago

Codecov Report

Merging #503 (ac5f747) into main (8fc7f7f) will decrease coverage by 3.16%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #503      +/-   ##
==========================================
- Coverage   74.47%   71.30%   -3.17%     
==========================================
  Files         317      308       -9     
  Lines       24121    22014    -2107     
  Branches       55     3489    +3434     
==========================================
- Hits        17964    15697    -2267     
+ Misses       6157     5420     -737     
- Partials        0      897     +897     
frmdstryr commented 1 year ago

I think the test is just accessing the member not triggering the factory? It would be good to call the layout method to get a little coverage.

MatthieuDartiailh commented 1 year ago

I meant to access the guide on an instance... I added calls to layout

bburan commented 1 year ago

What is the bug here? The only fix I see is to get rid of the import for QDir?

Ah, I got it. It has to do with dock_images not getting included in the distribution, right?

MatthieuDartiailh commented 1 year ago

It has to do with the fact that we need a valid package for path. And without the extra __init__ things were going really wrong.

bburan commented 1 year ago

@MatthieuDartiailh I see. For some odd reason I can't replicate the issue. On Python 3.9 at least it seems to automatically add a __init__.py file when installing to site-packages. Perhaps this bug is dependent on the version of setuptools? I've only just started learning about the new approach to Python packaging (i.e., putting everything in pyproject.toml) so I could be missing something obvious here.

Either case, I see no reason to hold off on this PR.

bburan commented 1 year ago

Just tested on a clean setup of Python 3.8 and I can replicate the bug. Applying this patch fixes the bug. Interesting. Probably a newer version of setuptools or setuptools_scm (that gets installed on Python 3.9) is smart enough to auto-create the __init__.py file? Either case, I can definitely confirm this patch fixes it.

MatthieuDartiailh commented 1 year ago

Thanks for testing thoroughly. I will merge and try to do a patch release ASAP.