-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Views for interval dict #34
Comments
Hello, Thanks for reporting this! With the exception of the "automatic updated content" of a view, what are the practical differences between a view and a list? I think it should be quite easy to implement a view object instead of a list for |
The view only references its source
Sure, this can indeed be easily achieved by inheriting from |
Thanks for these explanations! Feel free to submit a PR with these new "features" if you want, but keep me informed if you plan do to this, otherwise, I'll try to find some time to implement it by myself (I cannot say when it will be implemented, but be sure "it's not for soon" since I've a lot of other tasks to do before ;-)) |
I don't have any bandwidth for now, but I'll try to submit something in the next weeks; I'll let you know when I'll start working on it. |
Do you have pointers to existing subclasses of |
I had a quick look at from collections.abc import KeysView, ValuesView, ItemsView
class IntervalDict(MutableMapping):
def keys(self):
return KeysView(OrderedDict(sorted(self._items, key=_sort)))
def values(self):
return ValuesView(OrderedDict(sorted(self._items, key=_sort)))
def items(self):
return ItemsView(OrderedDict(sorted(self._items, key=_sort))) However, it would be considerably better to directly use an Another option, even though it's somehow hacky (the from collections.abc import KeysView, ValuesView, ItemsView
class IntervalDictBaseView:
def __init__(self, mapping):
self.__mapping = mapping
@property
def _mapping(self):
return sorted(self.__mapping._items, key=_sort)
class IntervalDictKeysView(IntervalDictBaseView, KeysView):
pass
class IntervalDictValuesView(IntervalDictBaseView, ValuesView):
pass
class IntervalDictItemsView(IntervalDictBaseView, ItemsView):
pass
class IntervalDict(MutableMapping):
def keys(self):
return IntervalDictKeysView(self)
def values(self):
return IntervalDictValuesView(self)
def items(self):
return IntervalDictItemsView(self) but I think that the first option with an |
Thank you! I prefer the first solution, but in that case, the ordering won't be kept in the views when the IntervalDict is modified. One possibility is indeed to change the internal structure to a sorted list (I don't see why an OrderedDict will be required here). Moreover, having an ordered list internally will be helpful to achieve better performance in some cases. That's something I planned a long time ago - to switch to an interval-tree-like structure internally - but I couldn't find time to work on it. |
My previous post was erroneous; the first option is not working as it is. It has the same drawback as the current implementation (with a
What do you refer to by "sorted list"? As You should consider using an If you think that the class can be implemented with an Let me know if you start working on it so we can synchronise ourself and avoid work duplication. |
Perhaps I misunderstood something ;-) As far as I know, At the time I wrote Create with [1, 3] -> 'a'; Internally, this will be stored as [1, 3] | [7, 8] -> 'a' + [5] -> 'b'. Insertion order will be difficult to provide in this case. That's mainly why the order in which items are returned in |
I totally agree that I think that preliminary was clear to you, but let me recall it anyway (just to be sure :) ). Now, there are two options under review:
Looks like you have a preference for the first option (and I do agree ;) ). My point is that this (probably) implies to rework the whole class; this is why it may be worth considering the second option anyway. But you are most likely to estimate the amount of work necessary ;) |
Hello, Thanks for these explanations. I think I get it now ;-) I indeed prefer the first option, which seems cleaner and more appropriate. However, since it requires to maintain the intervals sorted and, as such, requires a lot of changes, I think we can opt for the second option as a "temporary workaround". At some point, I hope to find time and courage to write an |
I was looking at the implementation of For example, the Am I right, or did I miss something? :-) |
Indeed, you're right; so none of options 1 or 2 will work out-of-the-box. It might be necessary to override |
I started thinking about this. I think we need, at least, to:
Does it seem right? I think we'll need a lot of unit tests to be sure that we do not miss something ;-) |
Hello, I found some time this afternoon to have a look at this issue. I've created a PR, see #35. Thank you! |
Closed by #35 |
Hi; thanks for that great library!
While using
portion.IntervalDict
, I noticed that it is not fully respecting the interface of thedict
class: the.keys()
,.values()
and.items()
methods should return view objects instead oflist
objects. It would be really helpful to have this mechanism implemented (it would slightly improve performances as well in some specific circumstances).Best.
The text was updated successfully, but these errors were encountered: