Implement a low-level interface for loaders to implement instead of TraversableResources

[jaraco]

In GitLab by @jaraco on Mar 24, 2020, 11:36

As discussed in #90, this MR attempts to implemented the proposed interface for loaders to supply more basic functionality for loading resources.

[jaraco]

In GitLab by @gregory.szorc on Apr 5, 2020, 11:48

Commented on importlib_resources/abc.py line 125

I assume instances are created via a sibling method to Loader.get_resource_reader()? If so, +1.

[jaraco]

In GitLab by @gregory.szorc on Apr 5, 2020, 11:48

Commented on importlib_resources/abc.py line 144

What does virtual mean here? Must the child packages also be actual Python packages or can any e.g. child directory be returned, even if it doesn't have a Python package / __init__.py?

[jaraco]

In GitLab by @gregory.szorc on Apr 5, 2020, 11:48

Commented on importlib_resources/abc.py line 161

Nit: I'd prefer a name like package_leaf_name or anything more specific than name.

[jaraco]

In GitLab by @gregory.szorc on Apr 5, 2020, 11:50

I only really looked at the interface as it pertains to the SimpleReader, as I think that's what I care about most since that is PyOxidizer's exposure to this problem space. I overall think the new interface is great. It is simple. It just needs a bit more formalization around semantics.

[jaraco]

In GitLab by @gregory.szorc on Apr 5, 2020, 11:51

Commented on importlib_resources/abc.py line 151

What are the rules for resources in child sub-directories? Can returned resources have e.g. / in their names?

[jaraco]

In GitLab by @gregory.szorc on Apr 5, 2020, 11:51

Commented on importlib_resources/abc.py line 158

Is resource allowed to have a / or \ character?

[jaraco]

In GitLab by @brettcannon on Apr 6, 2020, 13:21

Commented on importlib_resources/abc.py line 125

Would it be worth making this a protocol instead of an ABC?

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 11:36

Commented on importlib_resources/abc.py line 125

I assume instances are created via a sibling method to Loader.get_resource_reader()?

I was imagining that Loader.get_resource_reader would start to return TraversableReader objects where they previously returned ResourceReaders. Since TraversableReaders are both instances of SimpleReader and TraversableResources, they'd conform to both interfaces while satisfying backward compatibility.

Would it be worth making this a protocol instead of an ABC?

I'm not familiar with protocols, so I'd be ill-suited to make a value judgment or advise on best practices. I just read up on protocols (PEP 544) and the idea seems sound, but it's not obvious to me what the implications would be. I'm not sure what it means for a SimpleReader to define the .name property as it does now, if it were a protocol. I'd really like someone with more experience with protocols to help design it so I don't make n00b mistakes.

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 11:39

Commented on importlib_resources/abc.py line 144

I used this language from your prototype. What I mean here is any container of resources. I'll adjust the wording to be explicit.

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 11:41

added 1 commit

• 09786d6 - Update wording on SimpleReader.child_readers. Rename 'child_readers' to simply 'children'.

Compare with previous version

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 11:41

Commented on importlib_resources/abc.py line 144

Updated in 09786d6.

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 11:51

Commented on importlib_resources/abc.py line 161

Acknowledged.

My motivation here is twofold:

• When possible, classes, functions, properties, and methods should use single-word names and should rely as much as possible on their parent (containing class, module, etc.) to clarify the context.
• pathlib.Path objects expose a .name property. This property is attempting to capture that expectation. I'd like for pathlib.Path objects to be valid Traversable objects.

I'm not sure either of these motivations are relevant here. I need to think about this approach some more.

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 11:55

Commented on importlib_resources/abc.py line 151

I'm disinclined to provide any constraints unless I know those constraints add value. Too frequently, I see interfaces over-constrain in order to simplify the implementation around a specific use-case and inadvertently disallow future legitimate use-cases. It should be a Unicode string, but otherwise, I don't see any reason to constrain the names. If the underlying container can support / in the name, then I'm fine with this interface supporting this case. Is there any harm in that approach?

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 11:57

Commented on importlib_resources/abc.py line 158

It's up to the implementation to accept/reject specific values. Same reasoning as with resources().

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 15:08

added 1 commit

• 938a6b00 - Add compatibility for Python 2.7

Compare with previous version

[jaraco]

In GitLab by @jaraco on Apr 11, 2020, 15:10

added 1 commit

• b7e0288 - Add compatibility for Python 2.7

Compare with previous version

[jaraco]

In GitLab by @brettcannon on Apr 14, 2020, 15:33

Commented on importlib_resources/abc.py line 125

Protocol is an ABC, but when typing is done it's via duck/structural typing instead of nominal typing. That's it. You should be able to get away with just swapping out the base class.

IOW if someone types something as accepting SimpleReader and it inherits from Protocol then the object just has to match the API, but if SimpleReader inherits from ABC then type checkers will require the object inherit from SimpleReader.

[jaraco]

In GitLab by @jaraco on Apr 16, 2020, 17:07

Commented on importlib_resources/abc.py line 125

That's helpful. I'll experiment with Protocols. My instinct is that making Traversable a protocol is more important than making SimpleReader one. Regardless, if a Protocol is an ABC, then using an ABC is more constrained and it should be straightforward to convert it to a protocol at any time that it's needed, so I feel it's more important to get some viable implementation into Python 3.9 and relax constraints later (preferably before 3.9.0 final) if that adds value.

[jaraco]

In GitLab by @jaraco on Apr 16, 2020, 17:07

unmarked as a Work In Progress

[jaraco]

In GitLab by @jaraco on Apr 16, 2020, 17:08

@gregory.szorc I'm planning to merge this change as proposed. How likely is it you will be able to prototype a PyOxidizer branch against this branch to validate the behavior?

[jaraco]

In GitLab by @jaraco on Apr 16, 2020, 19:53

added 1 commit

• a4e5b12 - Add docstrings and type annotations. Fix references to parent and reader...

Compare with previous version