Skip to content

dune-sync-62 Dune: real and decimal(32,0)#66

Merged
bh2smith merged 5 commits intomainfrom
dune-sync-62
Nov 13, 2024
Merged

dune-sync-62 Dune: real and decimal(32,0)#66
bh2smith merged 5 commits intomainfrom
dune-sync-62

Conversation

@mooster531
Copy link
Copy Markdown
Collaborator

Closes #62

Comment thread src/sources/dune.py Outdated
@mooster531 mooster531 requested a review from bh2smith November 12, 2024 21:46
Copy link
Copy Markdown
Owner

@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.

I think this is great!

Would like to encapsulate the decision logic on types into one place that maintains the simplistic readability of the main flow.

Would also like to have an explicit unit test of the function introduced here.

Comment thread src/sources/dune.py
Comment thread tests/e2e_test.py
Comment thread src/sources/dune.py
Comment thread src/sources/dune.py
Comment thread src/sources/dune.py Outdated
@mooster531
Copy link
Copy Markdown
Collaborator Author

Added a test for the decimal parser and cleaned up those Unions.

@mooster531
Copy link
Copy Markdown
Collaborator Author

Integrated main (the JSON columns handling) now, please check again.

Comment thread tests/unit/dune_test.py Outdated
Copy link
Copy Markdown
Owner

@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.

Noice!

Co-authored-by: Benjamin Smith <bh2smith@users.noreply.github.com>
@bh2smith bh2smith merged commit 58a7bbf into main Nov 13, 2024
@bh2smith bh2smith deleted the dune-sync-62 branch November 13, 2024 07:49
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.

Dune: real and decimal(32,0)

2 participants