Skip to content
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

Changes done for the last evaluation #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

josojo
Copy link

@josojo josojo commented Feb 16, 2023

Changes dones for this evaluation:
https://docs.google.com/document/d/1jYOe5_SyvIxIRqXJ9bEoiWexBFP2pbhENDduENILGTA/edit#heading=h.t3w3qalgvh6v

Considering more data and avoiding NaN values

@josojo josojo requested a review from bh2smith February 16, 2023 15:41
Copy link

@bh2smith bh2smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good, although why would gas prices be null?

@@ -1,6 +1,6 @@
UNION_RAW_DATA_JOINED_WITH_PARAMETER = """with raw_data as (
select pa.uid, sell_amount, buy_amount, executed_sell_amount, executed_buy_amount, exchange, name, data, output_value_usd, gas_cost_usd_from_trader_contract, gas_cost_usd_from_trace_callMany, gas_price, sell_token_price, buy_token_price, eth_price, block_number from exchange prd inner join parameters pa on pa.uid = prd.uid

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant to this or, but this line is way too long. Is there any other fields that could be null we maybe want to exclude?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants