WIP: Minor code improvements #67
Conversation
|
Can one of the admins verify this patch? |
|
ok to test |
|
add to whitelist |
|
retest this please |
|
Seems like I can't test it locally. It keeps failing on import of |
|
That's unexpected. Doing a |
|
I just had to uninstall the module than reinstall it. I have no idea what happened before. I'm getting different errors now, but at least I've solved that problem. Edit: These are the errors I'm getting: https://gist.github.com/Solomon1732/d079b53b701aceabd2cffb3f47195e5a |
Use an emptry call to `super()` instead of hardcoding the arguments. Also use str.format instead of the % operator. This was chosen instead of f-strings since it allows to use the equivalent of %g.
Modified loop structures in some functions. Instead of constructing the lists one element at the time, iterators, genexprs, and list comprehensions are used in order to make the list building more efficient.
Modified functions' internal list handlings. Instead of using temporary lists or building lists by repeatedly appending items, the modified functions now use list comprehensions, `list.extend`, genexps, and iterators to more efficiently build lists. Removed some declarations of `object` as parent class. Modified `PriceLevelList.insert` to use `bisect.insort` to avoid having to sort the list at each item insertion. Replace explicit super() calls with no args calls Replaced explicit calls to `super()` with either the no arguments version or outright deleted. Minor refractoring to reduce code reduplication Minor refractoring to reduce code reduplication.
Changed % string operator occurances to f-string when possible, or to `str.format` when not.
Changed the structure of a few functions, namely orderbook's __str__ function. Reduces code duplication. Also changed _get_order_ids_iter and _get_price_level_iter.
Removed `object` as parent class from various classes. This is because it is redundant in python 3.
Changed function to use list comprehention instead of appending to the list one element at a time. Change how some functions work In `BlockCache.get_missing_in_range` used list comprehention instead of manually appending to a new list in a loop. In `OrderBook._get_order_ids` used `map` instead of manually yielding in a loop. In `MatchPriorityQueue` used a `namedtuple` as the element of the internal `queue` list. Used `any` with a generator expression in `contains_order` instead of the loop with `_, _, other_order_id, _` destructuring, utilizing the introduced `namedtuple`. In `insert` utilized the new `namedtuple` in the internal `cmp_items` function for maintainability and readability. Also in `insert` instead of creating a new sorted list for `self.queue` the queue is sorted in-place.
Used `next` with a default value of `None` instead of using `try-except` on `StopIteration`. Removed `u` prefix from string. The prefix is superfluous in python 3. Instead of checking `current_version` against individual numbers it is checked against a tuple of values. Change parameterized `super()` to empty `super()` Instead of calling `super()` with explicit parameter, an empty call to it is made instead.
Changed the raising of ValueError to TypeError when appropriate. Namely when an argument's type is checked. Likewise changed the tests to match the new exceptions thrown. In `bitcoinlib_main.py` moved some software structures out of a file's `with` statement in order to not hold the file open longer than necessary. In addition restructured a for-loop in a minor way in order to decrease the indentation level of the loop's body.
Modified PriceLevelList to utilize bisect for various methods. In addition, modified the class to raise IndexError manually in some cases so as to hide implementation details.
Added a comment to improve code readability and highlight the difference between the two if cases. Also made the two cases one-liners.
|
Latest commit was mainly to add a comment in order to prevent further issues like #68 being opened in the future upon code inspection. |
|
@devos50 in Currently the named tuple is this: @dataclasses.dataclass(eq=True, frozen=True)
class _QueueItem(NamedTuple):
retries: int
price: Price
is_ask: bool
order_id: OrderId
other_quantity: Any |
|
Good question. First, you should understand some basics of the matchmaking mechanism as implemented in AnyDex. In AnyDex, matchmakers send match notifications to traders that have outstanding buy and sell orders. When a trader receives such a notification, it adds an entry to the match queue of the associated order. The match queue is a priority queue that prioritizes orders with a high quality. Each entry in the match queue has a few fields as you implemented in the To answer your question, we prioritize first on retries, and then on the price (a better price will be prioritized). However, what constitutes the 'better' price depends on whether the order is a buy or sell order. This is the logic on line 33-42 in the match queue logic. Hopefully this provides sufficient information for you to continue 👍 |
|
So, just to see if I'm on the mark, is this example fitting for what you have in mind? def __eq__(self, other) -> bool:
return self.retries == other.retries and self.price == other.price
def __lt__(self, other) -> bool:
if self.retries < other.retries:
return True
elif self.retries == other.retries:
if self.match_order.is_ask():
return self.price < other.price
else:
return not (self.price < other.price)
else:
return False |
|
@Solomon1732 I think it is the correct logic, but you could consider adding a few unit tests to more thoroughly check/compare your solution 👍 |
|
@Solomon1732 any plans to finish this PR? |
|
Things happened and I completely forgot about this PR! I'm so sorry! I'll close it for now since in the near future I don't think I'll manage to push this through. |
Implement minor code improvements as discussed in #63
Fixes #63