[stdlib] Add `Dict.get(key) -> Optional[T]` and `Dict.get(key, default) -> T` methods

[martinvuyk]

added both functions to keep compatibility with python

fn get(self, key: K) -> Optional[V]:
    ...

fn get(self, key: K, default: V) -> V:
    ...

[gabrieldemarmiesse]

Also, if there is an issue related to Dict.get, don't hesitate to mention it here :) that will create a link between this pull request and the issue, it will help the maintainers doing the bookkeeping

[martinvuyk]

self.find(key) has already unit tests. But get(self, key: K, default: V) -> V doesn't have unit tests yet. Maybe you can add a few unit tests for this function in https://github.com/modularml/mojo/blob/nightly/stdlib/test/collections/test_dict.mojo ?

I didn't think of adding unit tests since it would be essentially testing Optional.or_else()

suggestion find is not a Python dict method. See https://docs.python.org/3/library/stdtypes.html#mapping-types-dict . What is does it playing the role of the get() method you just added. So maybe, to avoid having two methods doing the same thing, you could rename find() into get()?

I didn't read or make any proposals to change the API so I left it alone (I also think it could either be renamed or made a "private" method as _find)

[gabrieldemarmiesse]

I didn't think of adding unit tests since it would be essentially testing Optional.or_else()

You are right! That's also testing everything in the body of the function fn get(self, key: K, default: V) -> V:, which right now contains only return self.find(key).or_else(default), but might contain other pieces of code in the future. Unit tests are also here to ensure that the next contributor doesn't break your code.

It's very common for project to have tests covering all of their public APIs. Which means that if we don't add tests now, the next contributor can add some logic, assume that unit tests are present and merge this because the test suite is passing while they added broken code. So we'll likely want unit tests for all the public api, even for very simple functions. You can take a look at other simple public functions in the Mojo stdlib, it's extremely likely they'll have unit tests.

I didn't read or make any proposals to change the API so I left it alone (I also think it could either be renamed or made a "private" method as _find)

Indeed we can wait for the decision of someone from the modular staff here

[martinvuyk]

Added a short and simple unit test, Dict.find() didn't have one. Bunched both (find & get) in the same test since they are aliases for each other

[modularbot]

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.