-
Notifications
You must be signed in to change notification settings - Fork 9
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
Conversation
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.
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*/ |
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.
We shouldn't need to do this, there should already be a .gitignore
in your .venv
.
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.
there was not so I had to add it in.
Do you also mean that the .gitignore
I have should not be getting pushed?
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.
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).
tests/contrib/polars/test_client.py
Outdated
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.
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.
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.
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.
|
Related to this issue - #29
Features
TODO
polars_utils.py