-
-
Notifications
You must be signed in to change notification settings - Fork 36
Refractor some of FrozenList's functions and methods #712
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
base: master
Are you sure you want to change the base?
Changes from all commits
edcc406
9ac91b6
45f619a
d308a7a
a56030a
099c011
11b16d1
a17bd53
29acbc5
c5fb3d9
2b7b0aa
2f6e409
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Refractor methods `__len__`, `__iadd__`, `index`, `extend`, `append`, `count` and `__deecopy__` and add notes for `pop`, `append` and `extend` for contributors who are looking to | ||
| refractor anything further -- by :user:`Vizonex`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,12 @@ | |
| # distutils: language = c++ | ||
|
|
||
| from cpython.bool cimport PyBool_FromLong | ||
| from cpython.list cimport PyList_GET_SIZE | ||
| from cpython.long cimport PyLong_FromLong | ||
| from cpython.sequence cimport ( | ||
| PySequence_Count, | ||
| PySequence_InPlaceConcat, | ||
| ) | ||
| from libcpp.atomic cimport atomic | ||
|
|
||
| import copy | ||
|
|
@@ -31,9 +37,6 @@ cdef class FrozenList: | |
| if self._frozen.load(): | ||
| raise RuntimeError("Cannot modify frozen list.") | ||
|
|
||
| cdef inline object _fast_len(self): | ||
| return len(self._items) | ||
|
|
||
| def freeze(self): | ||
| self._frozen.store(True) | ||
|
|
||
|
|
@@ -49,7 +52,8 @@ cdef class FrozenList: | |
| del self._items[index] | ||
|
|
||
| def __len__(self): | ||
| return self._fast_len() | ||
| # Cython does less expensive calling if PyList_GET_SIZE is utilized | ||
| return PyList_GET_SIZE(self._items) | ||
|
|
||
| def __iter__(self): | ||
| return self._items.__iter__() | ||
|
|
@@ -71,20 +75,20 @@ cdef class FrozenList: | |
| if op == 5: # => | ||
| return list(self) >= other | ||
|
|
||
| def insert(self, pos, item): | ||
| def insert(self, *args): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably a bad idea.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem with insert is that it only does positional arguments. Hand-passing them off seemed to be the faster move.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what you're trying to achieve but here's a typical signature: https://docs.python.org/3/library/stdtypes.html#sequence.insert |
||
| self._check_frozen() | ||
| self._items.insert(pos, item) | ||
| self._items.insert(*args) | ||
|
|
||
| def __contains__(self, item): | ||
| return item in self._items | ||
|
|
||
| def __iadd__(self, items): | ||
| self._check_frozen() | ||
| self._items += list(items) | ||
| self._items = PySequence_InPlaceConcat(self._items, items) | ||
| return self | ||
|
|
||
| def index(self, item): | ||
| return self._items.index(item) | ||
| def index(self, *args): | ||
| return self._items.index(*args) | ||
|
|
||
| def remove(self, item): | ||
| self._check_frozen() | ||
|
|
@@ -96,22 +100,29 @@ cdef class FrozenList: | |
|
|
||
| def extend(self, items): | ||
| self._check_frozen() | ||
| self._items += list(items) | ||
| # Cython will generate __Pyx_PyList_Extend | ||
| self._items.extend(items) | ||
|
|
||
| def reverse(self): | ||
| self._check_frozen() | ||
| # Cython will do PyList_Reverse by default... | ||
| self._items.reverse() | ||
|
|
||
| def pop(self, index=-1): | ||
| # XXX: Current pop is impossible to refactor and may | ||
| # require the Cython maintainers to brainstorm a new idea. | ||
| self._check_frozen() | ||
| return self._items.pop(index) | ||
|
|
||
| def append(self, item): | ||
| self._check_frozen() | ||
| return self._items.append(item) | ||
| # Cython will generate an appropriate function for append | ||
| self._items.append(item) | ||
|
|
||
| def count(self, item): | ||
| return self._items.count(item) | ||
| # NOTE: doing self._items.count(item) Generates expensive call | ||
| # As for PyLong_FromLong it's a bit faster to call the direct C-API | ||
| return PyLong_FromLong(PySequence_Count(self._items, item)) | ||
|
|
||
| def __repr__(self): | ||
| return '<FrozenList(frozen={}, {!r})>'.format(self._frozen.load(), | ||
|
|
@@ -140,7 +151,8 @@ cdef class FrozenList: | |
|
|
||
| # Preserve frozen state | ||
| if self._frozen.load(): | ||
| new_list.freeze() | ||
| # faster to call .store directly rather than freeze() | ||
| new_list._frozen.store(True) | ||
|
|
||
| return new_list | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use RST, not markdown.