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

[documentation] Add example calls for the Crypto endpoints #564

Closed
wants to merge 1 commit into from
Closed

[documentation] Add example calls for the Crypto endpoints #564

wants to merge 1 commit into from

Conversation

GenusGeoff
Copy link
Contributor

@GenusGeoff GenusGeoff commented Jan 3, 2021

Added a couple of examples for Crypto Documentation. It needs some more work and a bit more meat, but this is possibly better than no documentation.

🎉 Hello there 🎉! Thanks for taking the time to contribute to Tiingo-Python!

Summary of Changes

Checklist

  • Code follows the style guidelines of this project
  • Added tests for new functionality
  • Update HISTORY.rst with an entry summarizing your change
  • Tag a project maintainer

Added a couple of examples for Crypto Documentation. It needs some more work and a bit more meat, but this is possibly better than no documentation.
@codecov
Copy link

codecov bot commented Jan 3, 2021

Codecov Report

Merging #564 (70ca832) into master (3fa3fe2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #564   +/-   ##
=======================================
  Coverage   94.73%   94.73%           
=======================================
  Files           5        5           
  Lines         228      228           
=======================================
  Hits          216      216           
  Misses         12       12           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3fa3fe2...70ca832. Read the comment docs.

@hydrosquall hydrosquall changed the title Update README.rst [documentation] Add example calls for the Crypto endpoints Jan 6, 2021
Copy link
Owner

@hydrosquall hydrosquall left a comment

Choose a reason for hiding this comment

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

Thanks for helping out @GenusGeoff ! I have 2 suggestions for us to make to both lines, but this gets us started on a great foot. After these changes are made, I'll be happy to bring this in.

As follow-ups for you or any future contributors, additional changes that should be documented before we can close out #520

  • Add an example for get_crypto_metadata
  • Add lines to the call showing all the available URL arguments for each method by looking at the code in the diff that introduced these features:

https://github.com/hydrosquall/tiingo-python/pull/340/files


.. code-block:: python
# NOTE: Crypto symbol MUST be encapsulated in brackets as a Python list!
``crypto_price = client.get_crypto_top_of_book(['BTCUSD'])``
Copy link
Owner

Choose a reason for hiding this comment

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

We can get syntax highlighting if we remove the backticks (``) and indent the python code, like so:

.. code-block:: python
    # NOTE: Crypto symbol MUST be encapsulated in brackets as a Python list!
    crypto_price = client.get_crypto_top_of_book(['BTCUSD'])

@@ -138,6 +138,20 @@ To receive results in ``pandas`` format, use the ``get_dataframe()`` method:

You can specify any of the end of day frequencies (daily, weekly, monthly, and annually) or any intraday frequency for both the ``get_ticker_price`` and ``get_dataframe`` methods. Weekly frequencies resample to the end of day on Friday, monthly frequencies resample to the last day of the month, and annually frequencies resample to the end of day on 12-31 of each year. The intraday frequencies are specified using an integer followed by "Min" or "Hour", for example "30Min" or "1Hour".


You can obtain top-of-book Cryptocurrency quotes from the ``get_crypto_top_of_book()`` method.
Copy link
Owner

Choose a reason for hiding this comment

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

Cryptocurrency should be lowercase, as it's not a proper noun.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @hydrosquall, I'm not sure if the changes I made are showing for you. I did make the requested changes, but I see no record of them. I might be lost in the GitHub ether here.

Copy link
Owner

Choose a reason for hiding this comment

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

Hi @GenusGeoff, thanks for checking in. Currently I only see the changes from December (screenshot here).

Is it possible that the changes are on your local git repository, but haven't been pushed up to Github yet?

Alternately, if you have the raw text for what you intended to add, feel free to post it as a comment on this PR, and I can help you get it added to the branch.

@GenusGeoff GenusGeoff marked this pull request as draft March 9, 2021 03:06
@GenusGeoff
Copy link
Contributor Author

My_changes.txt

NOTE: The segments would need to be copy-pasted as this file is behind the current one with the fundamental data added. (Or otherwise pulled, edited and merged).

@GenusGeoff GenusGeoff marked this pull request as ready for review March 13, 2021 17:51
@GenusGeoff GenusGeoff closed this Mar 24, 2021
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