Skip to content
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

Add dict.pop() #1989

Open
plajjan opened this issue Nov 15, 2024 · 3 comments
Open

Add dict.pop() #1989

plajjan opened this issue Nov 15, 2024 · 3 comments
Assignees
Labels
builtins Related to builtins enhancement New feature or request

Comments

@plajjan
Copy link
Contributor

plajjan commented Nov 15, 2024

We discussed adding .pop() to dict / Mapping in our weekly meeting.

We said we wanted this:

protocol Mapping[A(Eq),B] (Container[A], Indexed[A,B]):
# NO EXCEPTIONS!!!
    get         : (A,?B) -> ?B
    pop         : mut(A,?B) -> ?B

And these functions should not throw any exceptions - they should return None if the key is not found.

@plajjan
Copy link
Contributor Author

plajjan commented Nov 15, 2024

Hmm, so on second consideration, don't we want to have separate method for the get-with-default-value??

With the change as written in this issue and implemented in #1986, we end up having to None-check the result, even when we provide a default value - that seems suboptimal.

Like I feel the whole idea with a default value is that we know we get a non-optional value back. We can never statically enforce the type to be non-optional for a single get method where the default argument is optional, so I guess we need separate methods?

Something like this:

  • __getitem__(A) -> B but throws KeyError if A does not exist
  • get(A) -> ?B returns None if A does not exist
  • getdef(A, B) -> B returns B if A does not exist

and similar for pop and popdef then !?

@sydow @nordlander @ddjoh WDYT?

@nordlander
Copy link
Contributor

I agree, get with a default value is not the same as an optional get. One could merge the two by moving to the more elaborate signature

    get : [B(D)] => (A, Optional[D]) -> D

but that would require the planned support for unions as well as a lifted restriction in the constraint-solver. For later (perhaps).

The only thing about adding getdef is that def probably reads as definition to must Python programmers, not default. Maybe getopt could used instead?

@plajjan
Copy link
Contributor Author

plajjan commented Nov 15, 2024

I wouldn't read too much into the name. Can't say I would immediately feel getopt is more intuitive than getdef. I mean, sometimes you just have to learn what something means and not what you might guess from a few characters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins Related to builtins enhancement New feature or request
Development

No branches or pull requests

3 participants