-
Notifications
You must be signed in to change notification settings - Fork 21
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
Analysis stage #844
Analysis stage #844
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
1b81154
to
fdc975c
Compare
…o initialize session to drop all tables for testing
…and postgres diff
41412a5
to
dcc4ceb
Compare
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.
Overall it looks great to me! I agree with the proposed formatting & data flow. I am assuming that your tests ensure the trading pipeline still works, or that you tested it locally.
The change from using *fn_args
to fn_args
is fine with me, but I think this should be reflected everywhere (e.g. in ethpy we use the splats for smart_contract_read
& smart_contract_transact
). We don't want to have an inconsistent interface for the ethpy API.
We will be turning on the docstring linting soon (once we are done with these major tasks for the next competition), so you might want to just add the full docs to your new functions now to save a headache later. Your call.
This PR reworks the chainsync data to dashboard pipeline to move computation away from the dashboard and into a separate process.
Major Changes:
run_data_analysis.py
process to run analysis afteracquire_data
that focuses on computation.Minor Changes:
initialize_session
to drop existing tables if set to True for debugging.Breaking Change:
Bug Fixes:
smart_contract_preview_transaction
now takes in a block number argument to query the mock trade at that exact block. This helps solve a race condition where the preview could be mocking a position that's no longer there.TODO: