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

feat: custom subgrounds client for polars #43

Closed
wants to merge 14 commits into from

Conversation

Evan-Kim2028
Copy link

@Evan-Kim2028 Evan-Kim2028 commented Sep 17, 2023

Related to this issue - #29

from polars_client import SubgroundsPolars
from subgrounds.subgraph import FieldPath, Subgraph

# from polars_utils.py import *

import polars as pl

sg = SubgroundsPolars()

snx_endpoint = "https://api.thegraph.com/subgraphs/name/synthetix-perps/perps"

snx = sg.load_subgraph(
    url=snx_endpoint,
)

trades_json = sg.query_json(
    [
        # set the first parameter to a larger size to query more rows.
        snx.Query.futuresTrades(
            first=2500,
            orderBy="timestamp",
            orderDirection="desc",
            # where=[{"timestamp_lte": "1694131200"}],  # 1694131200 = 9/8/23
        )
    ]
)

Features

  • create a custom subgrounds client to support polars dataframes

TODO

  • uses custom polars utility functions to help convert from graphql response to polars dataframe. This is currently done, but need to work out importing functions from polars_utils.py

@0xMochan 0xMochan self-requested a review September 17, 2023 21:48
Copy link
Collaborator

@0xMochan 0xMochan left a comment

Choose a reason for hiding this comment

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

There's going to be some work before we get to a PolarsClient, we can set up a call to go over more about this!

Also, I've renamed some files to match some of our naming conventions and so forth. Lastly, checkout the CONTRIBUTING for the tools we use for managing the repo.

@@ -15,3 +15,4 @@ subgrounds.egg-info/

# apple
.DS_Store
.venv*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't need to do this, there should already be a .gitignore in your .venv.

Copy link
Author

Choose a reason for hiding this comment

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

there was not so I had to add it in.

Do you also mean that the .gitignore I have should not be getting pushed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The .gitignore is auto-generating if the .venv is created via python's built in venv module (which is also what poetry uses for generating environments).

subgrounds/contrib/polars/__init__.py Outdated Show resolved Hide resolved
subgrounds/contrib/polars/client.py Outdated Show resolved Hide resolved
subgrounds/contrib/polars/utils.py Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test file is not structure for pytest, it's more like a testing file. We can consider adding this to if __name__ == "__main__" in the main polars client file.

In terms of actual tests, if you aren't familiar with pytest, it might be a bit hard to follow how we structure tests in the tests directory. We can set a call for this or I can write a mock for how it could look like.

Copy link
Author

Choose a reason for hiding this comment

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

correct - test_client.py is simply a testing file to ensure that the functionality works. It does not cover any edge cases, bugs, or all pieces of the polars client such as testing any of the polars utils functions.

what kind of pytest tests would you recommend to implement, if any? I see a lot of existing tests for the current sync subgrounds, unsure if any of them would be good to copy.

@Evan-Kim2028
Copy link
Author

  • refactored function names, comments, and style for utils
  • added functional tests to see how the functions work

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