Feat/liquidity info #281
Merged
Feat/liquidity info #281
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adamantios
reviewed
Jun 19, 2024
Comment on lines
627
to
629
| if old_amounts is not None: | ||
| l0_start, l0_end = old_amounts[0], new_amounts[0] | ||
| l1_start, l1_end = old_amounts[1], new_amounts[1] |
There was a problem hiding this comment.
Let's convert this to a structure, for example, a dataclass called Liquidity that has the l0s and l1s.
Adamantios
reviewed
Jun 21, 2024
Comment on lines
112
to
116
| def update_liquidity_amounts(self, old_amounts: List[int], new_amounts: List[int]): | ||
| """Function to update the liquidity amounts""" | ||
| self.l0_start, self.l0_end = old_amounts[0], new_amounts[0] | ||
| self.l1_start, self.l1_end = old_amounts[1], new_amounts[1] | ||
| return |
There was a problem hiding this comment.
This won't be necessary if the old_amounts and new_amounts get replaced with an instance of this class.
Comment on lines
280
to
283
| if self.shared_state.mock_data is not None: | ||
| question_id = self.shared_state.mock_data.id | ||
| else: | ||
| raise ValueError("No mocked data information") |
There was a problem hiding this comment.
This is duplicated below, you may create a property for safely accessing the self.shared_state.mock_data.
There was a problem hiding this comment.
Without the BenchmarkingMockData there is no benchmarking mode so I guess this use-case is already covered in the code even before. No extra checks needed here.
Adamantios
reviewed
Jun 21, 2024
Comment on lines
627
to
628
| if liquidity_info is None: | ||
| self.context.logger.info("No market liquidity information.") |
There was a problem hiding this comment.
Here you can instead check if all the attributes are None. You can create a method in the dataclass for that.
Comment on lines
663
to
666
| liquidity_info.l0_start if liquidity_info is not None else None, | ||
| liquidity_info.l1_start if liquidity_info is not None else None, | ||
| liquidity_info.l0_end if liquidity_info is not None else None, | ||
| liquidity_info.l1_end if liquidity_info is not None else None, |
There was a problem hiding this comment.
Suggested change
| liquidity_info.l0_start if liquidity_info is not None else None, | |
| liquidity_info.l1_start if liquidity_info is not None else None, | |
| liquidity_info.l0_end if liquidity_info is not None else None, | |
| liquidity_info.l1_end if liquidity_info is not None else None, | |
| liquidity_info.l0_start, | |
| liquidity_info.l1_start, | |
| liquidity_info.l0_end, | |
| liquidity_info.l1_end, |
Comment on lines
99
to
111
| # Liquidity of tokens for option 0, before placing the bet | ||
| l0_start: int | ||
| # Liquidity of tokens for option 1, before placing the bet | ||
| l1_start: int | ||
| # Liquidity of tokens for option 0, after placing the bet | ||
| l0_end: int | ||
| # Liquidity of tokens for option 1, after placing the bet | ||
| l1_end: int | ||
|
|
||
| def __init__(self, l0_start, l1_start, l0_end, l1_end): | ||
| """Function to update the liquidity amounts""" | ||
| self.l0_start, self.l0_end = l0_start, l0_end | ||
| self.l1_start, self.l1_end = l1_start, l1_end |
There was a problem hiding this comment.
Suggested change
| # Liquidity of tokens for option 0, before placing the bet | |
| l0_start: int | |
| # Liquidity of tokens for option 1, before placing the bet | |
| l1_start: int | |
| # Liquidity of tokens for option 0, after placing the bet | |
| l0_end: int | |
| # Liquidity of tokens for option 1, after placing the bet | |
| l1_end: int | |
| def __init__(self, l0_start, l1_start, l0_end, l1_end): | |
| """Function to update the liquidity amounts""" | |
| self.l0_start, self.l0_end = l0_start, l0_end | |
| self.l1_start, self.l1_end = l1_start, l1_end | |
| # Liquidity of tokens for option 0, before placing the bet | |
| l0_start: Optional[int] | |
| # Liquidity of tokens for option 1, before placing the bet | |
| l1_start: Optional[int] | |
| # Liquidity of tokens for option 0, after placing the bet | |
| l0_end: Optional[int] | |
| # Liquidity of tokens for option 1, after placing the bet | |
| l1_end: Optional[int] |
Co-authored-by: Adamantios Zaras <[email protected]>
dvilelaf
reviewed
Jun 24, 2024
| @@ -618,6 +624,9 @@ def _write_benchmark_results( | |||
| ) | |||
| return | |||
|
|
|||
| if liquidity_info.l0_start is None: | |||
| self.context.logger.info("No market liquidity information.") | |||
There was a problem hiding this comment.
I see you are using liquidity_info on line 663-666. If this is None, shouldnt we handle this case differently?
…iquidity_info # Conflicts: # packages/packages.json
Adamantios
approved these changes
Jul 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature to include liquidity information at the benchmarking results output file including the initial and final liquidity values of the two options in the pool: option 0 and option 1.