Re: [Python-Dev] Unique loader per module
On 21 January 2018 at 01:56, Barry Warsaw wrote: > On Jan 05, 2018, at 05:12 PM, Nick Coghlan wrote: > >>I think the main reason you're seeing a problem here is because >>ResourceReader has currently been designed to be implemented directly >>by loaders, rather than being a subcomponent that you can request >>*from* a loader. >> >>If you instead had an indirection API (that could optionally return >>self in the case of non-shared loaders), you'd keep the current >>resource reader method signatures, but the way you'd access the itself >>would be: >> >>resources = module.__spec__.loader.get_resource_reader(module) >># resources implements the ResourceReader ABC > > BTW, just as a quick followup, this API suggestion was brilliant, Nick. It > solved the problem nicely, and let me add support for ResourceReader to > zipimport with only touching the bare minimum of zipimport.c. :) As API design rules of thumb go, "Prefer composition to inheritance" is one I've come to respect a *lot* :) Cheers, Nick. -- Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Unique loader per module
On Jan 05, 2018, at 05:12 PM, Nick Coghlan wrote: >I think the main reason you're seeing a problem here is because >ResourceReader has currently been designed to be implemented directly >by loaders, rather than being a subcomponent that you can request >*from* a loader. > >If you instead had an indirection API (that could optionally return >self in the case of non-shared loaders), you'd keep the current >resource reader method signatures, but the way you'd access the itself >would be: > >resources = module.__spec__.loader.get_resource_reader(module) ># resources implements the ResourceReader ABC BTW, just as a quick followup, this API suggestion was brilliant, Nick. It solved the problem nicely, and let me add support for ResourceReader to zipimport with only touching the bare minimum of zipimport.c. :) https://github.com/python/cpython/pull/5248 Cheers, -Barry pgpmPkp2zc3oF.pgp Description: OpenPGP digital signature ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Unique loader per module
Barry and I had a meeting at work today and we decided to go with Nick's idea of using a get_resource_reader(fullname) method on loaders. We aren't going to go with an ABC and simply depend on the method existing as implementing the API (and then returning None if the loader can't handle the specified module). I'll have a PR to update the docs out hopefully today for those that care. On Thu, 4 Jan 2018 at 23:14 Nick Coghlan wrote: > On 3 January 2018 at 06:35, Barry Warsaw wrote: > > Brett doesn’t like this, for several reasons (quoting): > > > > 1. redundant API in all cases where the loader is unique to the module > > 2. the memory savings of sharing a loader is small > > 3. it's implementation complexity/overhead for an optimization case. > > > > The second solution, and the one Brett prefers, is to reimplement zip > importer to not use a shared loader. This may not be that difficult, if > for example we were to use a delegate loader wrapping a shared loader. > > > > The bigger problem IMHO is two-fold: > > > > 1. It would be backward incompatible. If there’s any code out there > expecting a shared loader in zipimport, it would break > > 2. More problematic is that we’d have to impose an additional > requirement on loaders - that they always be unique per module, > contradicting the advice in PEP 302 > > We added module.__spec__.loader_state as part of PEP 451 precisely so > shared loaders had a place to store per-module state without have to > switch to a unique-loader-per-module model. > > I think the main reason you're seeing a problem here is because > ResourceReader has currently been designed to be implemented directly > by loaders, rather than being a subcomponent that you can request > *from* a loader. > > If you instead had an indirection API (that could optionally return > self in the case of non-shared loaders), you'd keep the current > resource reader method signatures, but the way you'd access the itself > would be: > > resources = module.__spec__.loader.get_resource_reader(module) > # resources implements the ResourceReader ABC > > For actual use, the loader protocol could be hidden behind a helper > function: > > resources = importlib_resources.get_resource_reader(module) > > For a shared loader, get_resource_reader(module) would return a new > *non*-shared resource reader (perhaps caching it in > __spec__.loader_state). > > For a non-shared loader, get_resource_reader(module) would just return > self. > > In both cases, we'd recommend that loaders ensure "self is > module.__spec__.loader" as part of their get_resource_reader() > implementation. > > Cheers, > Nick. > > -- > Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia > ___ > Python-Dev mailing list > Python-Dev@python.org > https://mail.python.org/mailman/listinfo/python-dev > Unsubscribe: > https://mail.python.org/mailman/options/python-dev/brett%40python.org > ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Unique loader per module
On 3 January 2018 at 06:35, Barry Warsaw wrote: > Brett doesn’t like this, for several reasons (quoting): > > 1. redundant API in all cases where the loader is unique to the module > 2. the memory savings of sharing a loader is small > 3. it's implementation complexity/overhead for an optimization case. > > The second solution, and the one Brett prefers, is to reimplement zip > importer to not use a shared loader. This may not be that difficult, if for > example we were to use a delegate loader wrapping a shared loader. > > The bigger problem IMHO is two-fold: > > 1. It would be backward incompatible. If there’s any code out there > expecting a shared loader in zipimport, it would break > 2. More problematic is that we’d have to impose an additional requirement on > loaders - that they always be unique per module, contradicting the advice in > PEP 302 We added module.__spec__.loader_state as part of PEP 451 precisely so shared loaders had a place to store per-module state without have to switch to a unique-loader-per-module model. I think the main reason you're seeing a problem here is because ResourceReader has currently been designed to be implemented directly by loaders, rather than being a subcomponent that you can request *from* a loader. If you instead had an indirection API (that could optionally return self in the case of non-shared loaders), you'd keep the current resource reader method signatures, but the way you'd access the itself would be: resources = module.__spec__.loader.get_resource_reader(module) # resources implements the ResourceReader ABC For actual use, the loader protocol could be hidden behind a helper function: resources = importlib_resources.get_resource_reader(module) For a shared loader, get_resource_reader(module) would return a new *non*-shared resource reader (perhaps caching it in __spec__.loader_state). For a non-shared loader, get_resource_reader(module) would just return self. In both cases, we'd recommend that loaders ensure "self is module.__spec__.loader" as part of their get_resource_reader() implementation. Cheers, Nick. -- Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
[Python-Dev] Unique loader per module
We have what I think is one last design question for importlib.resources. https://gitlab.com/python-devs/importlib_resources/issues/49 The problem is that the ResourceReader ABC omits the package from the function signatures, so that on a compatible loader, you only need to specify the resource you are interested in. This is fine for file loaders because every package will have a unique loader instance associated with it, so it will know which package the requested resource is homed to. But AFAICT, there is no specification or requirement in Python that every module/package have a unique loader instance. In fact, it’s not unreasonable given some of the text in PEP 302 to think that loader instances can be shared. The PEP says "In many cases the finder and loader can be one and the same object: finder.find_module() would just return self” and you aren’t going to typically have a unique finder per module, so that would implied a shared loader per finder. We even have an existence proof in the zip importer: >>> import test.test_importlib.zipdata02 >>> import sys, os >>> sys.path.insert(0, >>> os.path.join(os.path.dirname(test.test_importlib.zipdata02.__file__), >>> 'ziptestdata.zip')) >>> import ziptestdata.two >>> import ziptestdata.one >>> ziptestdata.one.__spec__.loader == ziptestdata.two.__spec__.loader True The issue above proposes two solutions. The first is to change the ABC so that it includes the package argument in the ABC method signatures. That way, a shared loader will know which package the requested resource is relative to. Brett doesn’t like this, for several reasons (quoting): 1. redundant API in all cases where the loader is unique to the module 2. the memory savings of sharing a loader is small 3. it's implementation complexity/overhead for an optimization case. The second solution, and the one Brett prefers, is to reimplement zip importer to not use a shared loader. This may not be that difficult, if for example we were to use a delegate loader wrapping a shared loader. The bigger problem IMHO is two-fold: 1. It would be backward incompatible. If there’s any code out there expecting a shared loader in zipimport, it would break 2. More problematic is that we’d have to impose an additional requirement on loaders - that they always be unique per module, contradicting the advice in PEP 302 The reason for this is third party finder/loaders. Sure, we can fix zipimport but any third party finder/loaders could have the same problem, and they’d be within their rights given the current specification. We’d have to prohibit that, or at least say that any third party finder/loaders that shared their loader can’t implement ResourceReader (which would be the practical effect anyway). I think that would be a shame. So while I agree with Brett that it’s uglier, and that once we decide we’re essentially locked into the API, I don’t see a whole lot of options. Thoughts, feedback, suggestions are welcome. -Barry signature.asc Description: Message signed with OpenPGP ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
