-
Notifications
You must be signed in to change notification settings - Fork 54
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
Allow to use ijson package by a relative import #93
Comments
I'll answer quickly now, as I'm about to go on leave for a couple of weeks: not entirely closed to the idea, but given the additional maintenance burden it would require more work to ensure: a) all imports have been taken into account (C extension?) and b) we don't accidentally add an absolute import in the future |
Hi rtobar, I'm happy to know that you're going on vacation and I wish you all the best... I have already taken care of the changes for the python code, on the other hand, for the C code, nothing has been done and I am not able to do it... And I don't know how to test all this part of the code in C... We'll talk about it after your return, see you soon. |
@prrvchr thanks for the wait. I have been thinking more about this, and I think I'll pass. The patch you produced looks good, and I experimented with the necessary changes in the C backend too, which wasn't too difficult either, so making these initial changes isn't an issue. For reference, this seems to work for the C backend: diff --git a/ijson/backends/yajl2_c/module.c b/ijson/backends/yajl2_c/module.c
index 581047c..85d202f 100644
--- a/ijson/backends/yajl2_c/module.c
+++ b/ijson/backends/yajl2_c/module.c
@@ -107,7 +107,8 @@ MOD_INIT(_yajl2)
INIT_ENAME(end_array);
// Import globally-used names
- PyObject *ijson_common = PyImport_ImportModule("ijson.common");
+ PyObject *ijson_common = PyImport_ImportModuleLevel("common", PyModule_GetDict(m), NULL, NULL, 2);
+ X_N(ijson_common, MOD_VAL(NULL));
PyObject *decimal_module = PyImport_ImportModule("decimal");
X_N(ijson_common, MOD_VAL(NULL));
X_N(decimal_module, MOD_VAL(NULL)); What I do worry about is maintainability: unless we have a good way to automatically check that this support isn't broken, we'll eventually break it, invalidating any changes we do now. And I am not really willing to put that much effort into developing such tests. They would ideally involve copying all the So all in all I don't think it's a bad idea, but I'm not ready to spend the effort that would be required to implement it properly. If you'd like to give it a go then I could review any proposed changes, but otherwise let me know and I'll close this ticket as not planned. |
I would be happy to participate and try to finish the integration, and I understand the need to integrate these changes into the tests. |
Hi,
I am using ijson package to stream json data coming from Request API.
I do this in a LibreOffice/OpenOffice extension (a file with the oxt extension) in which is a copy of the ijson package.
In order to remain independent of the system on which this extension is installed, I try if possible to use relative imports on my necessary python packages.
This is only possible if the python package you are trying to import also only uses relative imports.
This mode of use brings several improvements:
I only see advantages in it, and it should normally also work with Python version 2.7, and in fact the changes are minimal, they are visible on this link.
Anyway thanks again for your API.
The text was updated successfully, but these errors were encountered: