Skip to content

Add a partial, covariant Mapping type #47

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

Closed
wants to merge 4 commits into from
Closed

Conversation

harahu
Copy link

@harahu harahu commented Jun 3, 2022

Addresses #5

@harahu
Copy link
Author

harahu commented Jun 3, 2022

@srittau Decided to make an attempt at implementing the protocol we discussed a while ago.

Is there a checklist for what needs covering in a PR to typing_extensions?
I'm thinking in terms of docs, testing and the like.

@srittau
Copy link
Collaborator

srittau commented Jun 3, 2022

Side note: Depending on where #45 goes, this might be better suited for the new submodule.

@harahu
Copy link
Author

harahu commented Jun 3, 2022

Side note: Depending on where #45 goes, this might be better suited for the new submodule.

I agree. I was considering basing this PR off of your branch, but was unsure what the likely timeline for merge (and change potential) is like, so decided to just get started here.

@JelleZijlstra
Copy link
Member

The main typing-extensions namespace should not contain names that don't have a prospect of going into typing. Potentially this could go into typing_extensions.util if and when we add that.

Aside, I don't like the name much.

@gvanrossum
Copy link
Member

FWIW 'util' or 'utils' is a terrible name for a submodule. (And as I wrote in #5, I am not in favor of this PR in general, but I'll defer to Jelle+Sebastian.)

@harahu
Copy link
Author

harahu commented Jun 4, 2022

The main typing-extensions namespace should not contain names that don't have a prospect of going into typing. Potentially this could go into typing_extensions.util if and when we add that.

See comments above. This is the intention. Was just eager to get started. Hope you don't mind.

Aside, I don't like the name much.

I agree. The name is the part I struggle with the most. I have several other ideas, all of which I have trouble with:

  • PartialMapping
  • PartialMappingCo
  • MappingLike
  • MappingLikeCo

Suggestions welcome, in other words.

@harahu
Copy link
Author

harahu commented Jun 4, 2022

FWIW 'util' or 'utils' is a terrible name for a submodule. (And as I wrote in #5, I am not in favor of this PR in general, but I'll defer to Jelle+Sebastian.)

It would be constructive to provide an alternative to the claimed terrible name.

@gvanrossum
Copy link
Member

FWIW 'util' or 'utils' is a terrible name for a submodule. (And as I wrote in #5, I am not in favor of this PR in general, but I'll defer to Jelle+Sebastian.)

It would be constructive to provide an alternative to the claimed terrible name.

Sorry, you're right. In my experience in any large code base there are dozens of leaf modules named util[s], and the name just doesn't convey any meaning. Worse, if you are tempted to write from foobar import utils now you have a utils global that could have been imported from many different places.

Even if we called it typing_utils that would be better. Depending on the intended use of this package we could name it more specifically but I haven't followed what it's for -- @JelleZijlstra or @srittau could probably comment. (Not clear that it belongs in this PR's discussion, except you alluded that putting it in such a submodule was your intention.)

@AlexWaygood
Copy link
Member

AlexWaygood commented Jun 4, 2022

Even if we called it typing_utils that would be better.

I'm the person who suggested utils as a name, and I see your point — typing_utils sounds good to me!

But yeah, this is off-topic for this thread.

@harahu harahu mentioned this pull request Jun 4, 2022
@harahu
Copy link
Author

harahu commented Jun 4, 2022

(Not clear that it belongs in this PR's discussion, except you alluded that putting it in such a submodule was your intention.)

Yeah, sorry for encouraging continuing the conversation here. I made a comment mentioning this conversation in #45, in order to make the relevant conclusions visible.

@tmke8
Copy link

tmke8 commented Jul 15, 2022

Isn't this basically already covered by the SupportsItems protocol in #45 ?

@harahu
Copy link
Author

harahu commented Aug 1, 2022

Isn't this basically already covered by the SupportsItems protocol in #45 ?

This protocol would be a subtype of SupportsItems, but requires more from the implementer than just items, so no.

@JelleZijlstra
Copy link
Member

Closing this following the policy to only include things that are in CPython or in a PEP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants