pyotherside
pyotherside copied to clipboard
Fixes PyOtherSideQtRCImporter for submodule imports
It looks like the importer fix for python 3.12 in #131 does not cover imports of submodules. In moment we use importNames() like this:
addImportPath("qrc:/src")
importNames("backend.qml_bridge", ["BRIDGE"], () => { [...] });
This stopped working with the upgrade to python 3.12, #131 did not fix it. When used for a submodule, the path variable is set to the value of __path__ from the parent package according to the reference. By removing this path is None condition, it works again. Not sure though, if this MR will break other usecases though.
@apollo13: could you please test this against your setup?
Puh will see about testing that, I will try over the weekend. Can you check what path is in your case? Maybe it would be "more" correct to restore the initial check: https://github.com/thp/pyotherside/pull/131/files#diff-453f8c4ed0f10bda6f41ccfd2ab26d7bb45ecc92355ac956b53c75ff9eb1962aL25
So essentially:
if path is None or all(x.startswith('qrc:') for x in path):
In our application the path parameters for different cases look like this:
backend -> None
backend.qml_bridge -> ['qrc:/src/backend']
backend.models.model_item -> ['qrc:/src/backend/models']
cffi._pycparser -> ['/media/data/code/moment/venv/lib/python3.12/site-packages/cffi']
So excluding non qrc paths makes sense. Restoring this former check works, too, so I'll adjust this PR. Thanks for the hint.
Yeah, that makes sense! Thank for double checking.
@apollo13 @timegrid Is this ready to go in?
I didn't get around to test it. But since it restores the original check that I dropped and reallows sub imports it is ready imo
On Fri, May 31, 2024, at 08:38, Thomas Perl wrote:
@apollo13 https://github.com/apollo13 @timegrid https://github.com/timegrid Is this ready to go in?
— Reply to this email directly, view it on GitHub https://github.com/thp/pyotherside/pull/134#issuecomment-2141326600, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAT5C7H5IV7U6G2GKHSF3LZFALHTAVCNFSM6AAAAABHCXUEBGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBRGMZDMNRQGA. You are receiving this because you were mentioned.Message ID: @.***>
I agree. Additionally some moment users tested a patched version for a while, and everything seems fine with the imports.
Any chance, this fix could be released? For moment on archlinux we still need to provide an AUR package instead of the extra package. On other distros with python >= 3.12, it probably needs to be compiled manually until then.
Any chance, this fix could be released? For moment on archlinux we still need to provide an AUR package instead of the extra package. On other distros with python >= 3.12, it probably needs to be compiled manually until then.
This is now released as 1.6.2