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

qt: fix loading dock images #503

Merged
merged 3 commits into from
Aug 16, 2022
Merged

Conversation

MatthieuDartiailh
Copy link
Member

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

We test directly the loading since we cannot test D&D on the dock area
@MatthieuDartiailh
Copy link
Member Author

ping @bburan since you worked on the previous patch

@codecov-commenter
Copy link

codecov-commenter commented Aug 12, 2022

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
Copy link
Contributor

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
Copy link
Member Author

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

@bburan
Copy link
Contributor

bburan commented Aug 12, 2022

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
Copy link
Member Author

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
Copy link
Contributor

bburan commented Aug 15, 2022

@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
Copy link
Contributor

bburan commented Aug 15, 2022

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
Copy link
Member Author

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

@MatthieuDartiailh MatthieuDartiailh merged commit 6276cbb into main Aug 16, 2022
@MatthieuDartiailh MatthieuDartiailh deleted the fix-dock-area-image-loading branch August 16, 2022 07:07
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.

None yet

4 participants