WIP: collections as dicts

[chaselgrove]

Why is this refactoring not working? Does it have something to do with attrs?

[kyleam]

Why is this refactoring not working?

I didn't try anything with your example, so I don't know how exactly that fails but...

Does it have something to do with attrs?

... if it does have something to do with attrs, it's something more particular to how we use it, because this seems to work as expected:

import attr


@attr.s
class A(object):
    _a = attr.ib(default=attr.Factory(list))

    @property
    def a(self):
        return self._a

    @a.setter
    def a(self, value):
        self._a = value


x = A([0, 1])
print(x.a)
x.a = [0, 2]
print(x.a)

[0, 1]
[0, 2]

[chaselgrove]

Thanks, @kyleam. The test suite fails on a number of tests, but not in ways that were obvious as to the root cause.

@yarikoptic, can you shed light on this? Or suggest how I can create getters and setters for DebianDistribution.packages?

[yarikoptic]

Even without looking into failures: well, we do quite in a few places rely on exploring all those attr's attributes for the classes. E.g. git grep __attrs_attrs__ to see at least some of those spots. Here you are renaming packages into _packages, so saving into the spec would save it as _packages. And loading existing specs with packages would fail too.

This example doesn't show though the purpose of making packages into a property. Do you need to react on get or set for it, and how? Depending on your goal, see into using attr's functionality such as
Validators or Converters

[yarikoptic]

ah, doh -- re "purpose" the title said it ;-) So, you would like to convert them to be a dict instead of a list... we discussed it a bit during the last call. Do I remember correctly that the main goal is to make lookups (for diff and otherwise) more efficient so we don't need to iterate?

I guess the best would be to define a custom class to be used here instead of list (atm via TypedList) which would provide necessary abstraction and which we would "manually" handle upon saving/loading into a spec file (to store it there as a list). I would have probably subclassed dict so we could react while loading/saving to store it as a list of values. But I am not sure what other parts might rely on those to be just a list so iteration through it should then be done through values not keys... Since some other classes would still be regular lists, I would have probably created some adapter for now like dictlist_values_iterator which would do the right job depending on what it is to iterate over. Eventually we could get rid of it whenever all classes would converge if we see it as a way to go forward.

[chaselgrove]

@kyleam it looks like attrs uses the declaration of packages (via TypedList, which isn't a class but a function that returns an attr.ib()) to require it to be present at instantiation, so my attempt hid packages from attrs and it choked on DebianDistribution.init(). And also where we do introspection via __attrs_attrs__.

@yarikoptic my main purpose here was to express an idea by refactoring DebianDistribution -- I wanted to change its internals without affecting its behavior, but attrs limits our options here. We also run into problems because at some point(s?) we redefine packages completely. This is the reason we had to resort to working with __attrs_attrs__ in at least one case.

I'm rethinking my strategy and I suspect it might be better if the first step is not working on DebianDistribution's internals but rather on its interface, and to try to be stricter about how we can interact with it. This should also keep us from doing things that require crazy workarounds in other areas.