actonlang / acton

The Acton Programming Language
https://www.acton-lang.org/
BSD 3-Clause "New" or "Revised" License
80 stars 7 forks source link

Review dicts #1138

Open plajjan opened 1 year ago

plajjan commented 1 year ago

Seemingly not implemented: (should we implement?)

plajjan commented 1 year ago

@nordlander @sydow do we need to test the different types of things in dicts, like is it meaningful to test dict[str, str] and repeat same test for a dict[str, int] and dict[int, int]?

plajjan commented 1 year ago

Also, how do I compare two dicts?

    a = {"a": 1}
    b = {"a": 1}
    if a != b:
       print("a != b")
       await async env.exit(1)

this checks if a and b are references to the same thing, like same address... but how can I compare their content?

I've used repr(a) != repr(b).. but it feels expensive!?

And to verify, I think equality on lists does compare the contents of the list rather than its identity, right @nordlander @sydow ?

sydow commented 1 year ago

The bug you found in equality tests for dictionaries (which should be fixed any minute) shows that you should do similar tests for different types of keys and values. I am sure I tested equality for dictionaries but used ints as keys and not strs, and this bug didn’t appear.

Björ

On 28 Dec 2022, at 10:59, Kristian Larsson @.***> wrote:

@nordlander @sydow do we need to test the different types of things in dicts, like is it meaningful to test dict[str, str] and repeat same test for a dict[str, int] and dict[int, int]?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

plajjan commented 1 year ago

Similar to #1132, should we add the missing methods? @nordlander @sydow

sydow commented 1 year ago

This comment touches also on questions raised in other issues. They all come down to "Why is this method not supported in Acton as in Python". They all (?) have similar answers that go back to the days when we first implemented protocols. This was a tentative move and we did a first version, and then time and funding ran out and we have not followed up.

One guiding idea was to let our protocols be inspired by Python's abstract base classses, both for collections (https://docs.python.org/3/library/collections.abc.html) and for numbers (https://docs.python.org/3/library/numbers.html).

An early problem with collections was that methods pop and clear were present in several mutable base classes (sets, mappings and sequences). At that point we didn't want to introduce too many interrelated protocols, so we left pop and clear for the future. Later, when we did protocols for numbers, we had to give up and introduce separate protocols Plus, Minus, Times and Div, each with only one operator (and the corresponding incremental operator).

As regards copy there has been much debate in Python whether to include it in various base classes or not. To cite just one message in this debate, Guido van Rossum says [https://bugs.python.org/issue22101#msg224430]

You need to learn when to give up. :-) I wasn't the one who added a copy() method to other containers. I personally despise almost all uses of "copying" (including the entire copy module, both deep and shallow copy functionality). I much prefer to write e.g. list(x) over x.copy() -- when I say list(x) I know the type of the result.

Perhaps it is time to think carefully about how many (and which) protocols to have as builtins before we just add a subprotocol containing pop and clear (and copy). In general, I don't think that it is a good idea to add all functions that exist in Python (this is not to say that we don't miss some useful functions at present).

How should we go forward with this?

plajjan commented 1 year ago

I'm actually quite okay with our dicts overall. I started reviewing lists where I felt the lack of index and pop was painful. I just continued with dicts for completeness. I don't think python is some golden reference that we must follow. Things should make sense in Acton. We do need to answer how to clear and copy dicts though?

I have never used .clear() in python so can't say I find it that useful... But I also don't see why it would be so hard to implement. Am I correct in understanding that you are more concerned with the protocol hierarchy than the actual (C) function for doing clear and others?

.copy() seems useful but many times I think beginner users are fooled by shallow copies and if there is another way to (deep?) copy a dict then that's fine too, we just need to tell our users how.

.pop() feels natural to me though. It can of course be emulated by a get on a key followed by del, but why not support .pop()?

If this is a (base class) protocol thing, should we discuss this as a generic topic across all our collection types rather than per method?

plajjan commented 1 year ago

@sydow we already support .copy() on lists. We could instead tell users to do b = list(a) to get a (deep?) copy of the list, right? and then we could remove list.copy() for consistency and less confusion...

plajjan commented 1 year ago

@nordlander and I have discussed deep vs shallow copy in another meeting. Just want to jot down the outcome of that... AFAIR we like the ideas of Rossum that we don't need a .copy() function but should encourage c = dict(x) to get a copy of x. It should be a shallow copy though. I initially argued for a deep copy because I think that's often what people want, but it doesn't make sense. Johan had some specific examples that I can't remember the details of but whatever it was, it convinced me. Maybe that you can have actor references in a dict and getting a copy of an actor is like.. wut? What would that even mean, we can't read the private state of another actor and so can't take a copy of it.

I think we should have some way of doing deep copy though. Should that be a separate module / function? At least as interrim solution I guess that would be fine.