Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

s2t2
Copy link
Contributor

@s2t2 s2t2 commented Feb 19, 2022

Provides users with an option to return client responses in pandas.DataFrame format.

Supercedes #1133

@s2t2 s2t2 mentioned this pull request Feb 19, 2022
@s2t2
Copy link
Contributor Author

s2t2 commented Feb 19, 2022

Screen Shot 2022-02-19 at 12 46 59 PM

Copy link

@0ex-d 0ex-d left a comment

Choose a reason for hiding this comment

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

Just curious why redeclare
klines_df = historical_klines_response_df

Comment on lines 14 to 15
klines = historical_klines_response
assert len(klines) == 500
Copy link

Choose a reason for hiding this comment

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

Suggested change
klines = historical_klines_response
assert len(klines) == 500
assert len(historical_klines_response) == 500

def test_historical_klines(historical_klines_response_df):
klines_df = historical_klines_response_df
assert len(klines_df) == 500
assert isinstance(klines_df, DataFrame)
Copy link

Choose a reason for hiding this comment

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

Nice!

@s2t2
Copy link
Contributor Author

s2t2 commented Feb 20, 2022

Hi @mrbaguvix thanks for the review.

Just curious why redeclare klines_df = historical_klines_response_df

This was mostly for brevity in reading the test scenarios. But it's not necessary and the long variable name can be used instead.

@@ -0,0 +1 @@
pandas

Choose a reason for hiding this comment

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

I think, you should define version range: pandas>=n.n.n<n+1

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +1 to +4

import pytest
import requests_mock

Choose a reason for hiding this comment

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

Are you shure, that this file should be stored in project root?
And add module's description

Copy link
Contributor Author

@s2t2 s2t2 Mar 21, 2022

Choose a reason for hiding this comment

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

Confest in the root directory is a best practice to help us avoid adding init files to the test directory. It facilities local imports, especially as they pertain to testing.

https://stackoverflow.com/questions/34466027/in-pytest-what-is-the-use-of-conftest-py-files

Test root path: This is a bit of a hidden feature. By defining conftest.py in your root path, you will have pytest recognizing your application modules without specifying PYTHONPATH. In the background, py.test modifies your sys.path by including all submodules which are found from the root path.

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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.