-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Use package-relative imports to avoid collisions with user code #382
Comments
Hi! Great to see you here. My pleasure! You should not name file/module as another module. Because it will hide original module by local one. In your case please rename atproto_firehose.py to something else. Let me know will it help or not |
I mean I understand the problem but using a relative path is not the right approach here. IMHO. atproto package is just a single file project which only provides shortcuts to other packages. Probably it covers 80% of import needs for developers but not 100%. I mean, for example, if you want to use something deeper or cover all the code with type hints you may need to import additional things. And you will be forced to import some package directly. For example atproto_firehose one. And the problem described in this issue will hit you again. |
Hey, good to see you too, thanks for the reply! Users will have all sorts of modules and filenames of their own, and they install all sorts of different packages from pip. You can't know ahead of time which files your users will have, and they can't know ahead of time which files you have. It's very unusual, and not really reasonable, to require users to know and avoid all filenames that any of their dependencies use, either at the beginning or on an ongoing basis as everyone adds new files over time. Fortunately, package-relative imports solve this by giving every package its own namespaces! No more filename collisions. I know it will probably be an annoying chore to update all of your imports, but package-relative imports are generally expected in the ecosystem, and definitely worthwhile so that your users don't hit problems like these. |
Sure, but I can import If all of your internal imports are global, your package immediately breaks when I first try to use it, and I'm forced to rename files in my project to fix that breakage, which is unnecessary and a bad experience. |
Once again, package-related imports will save you only when you will perform “import atproto”. And even in this case, you are not allowed to create “atproto.py” This is so typical and annoying Python mistake. I personally sometimes create, for example, “json.py” and can't use a built-in json parser XD |
Oh we have desync in replies |
Right! A big part of the difficulty here is that you put a bunch of People expect that if they install the (As background, out of curiosity, I looked at a handful of popular open source Python libs - requests, click, attrs, flask, cryptography, dateutil, babel, bases, docutils, dns - and they all use package-local imports, either relative with |
I definitely get that this won't be easy to change! Especially if your users are currently importing from these top-level |
Yeah, this is my first project where I changed the structure to multi-top-level packages. The idea was to make it completely indented and maybe even publish it on PyPI separately. So the user can cherry-pick only the necessary part of the SDK. Like we have in the NPM ecosystem. But they have namespaces for the packages... and I am not sure that the current state of SDK is fully separated into independent packages. Firehose depends on the client, the client depends on core, etc... but you can probably install just the client without firehose (in the world where SDK is published to many PyPI packages). That's why all imports are not relative. Yeah, right now we have only one PyPI package. So I need to think about the future of it and do we need any action here Thank you for raising this! |
And yeah because of missed namespaces in the Python ecosystem, all packages have the same prefix “atproto_”. But unfortunately it could easily collide with users' codebase :( |
Understood! Yeah that's always a tension with a big project. Breaking it up into separate packages seems like a good idea for managing the complexity...but that moves some of the complexity onto your users to handle, ie understanding which of the many separate packages do they need for what they're trying to do. Still worthwhile sometimes! But complicated. |
Hi @MarshalX et al! First off, thanks for building and maintaining this library. It's great!
When I first tried it just now, I immediately hit an
ImportError
. I have anatproto_firehose.py
module in my project's root, and atproto doesn't use package-relative imports, so it tried to import its ownatproto_firehose
, got mine instead, and choked:You probably want to make all of your internal imports package-relative by adding a
.
to prevent this. Eg in this case, you'd dofrom .atproto_firehose import ...
The text was updated successfully, but these errors were encountered: