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

Feature/fundamentals #581

Merged
merged 1 commit into from
Feb 12, 2021
Merged

Conversation

kaessert
Copy link

@kaessert kaessert commented Feb 6, 2021

Summary of Changes

This PR will add three fundamental-data endpoints

@hydrosquall

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

@codecov
Copy link

codecov bot commented Feb 6, 2021

Codecov Report

Merging #581 (a2cfbc7) into master (820dfa0) will increase coverage by 0.55%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #581      +/-   ##
==========================================
+ Coverage   94.73%   95.29%   +0.55%     
==========================================
  Files           5        5              
  Lines         228      255      +27     
==========================================
+ Hits          216      243      +27     
  Misses         12       12              
Impacted Files Coverage Δ
tiingo/api.py 95.00% <100.00%> (+0.69%) ⬆️

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 820dfa0...a2cfbc7. Read the comment docs.

@kaessert kaessert force-pushed the feature/fundamentals branch 4 times, most recently from cea192b to b76f269 Compare February 6, 2021 16:35
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.

@atarax thank you for this! This looks great, I appreciate the detail on the tests + docstrings.

2 questions before merging this:

  1. Would you like to be added to the Contributors.rst?
  2. Would you be interested in adding a section to the README.rst showing people basic usage of these new methods you've added? If not , we can add an issue for someone else to add examples.

@kaessert
Copy link
Author

kaessert commented Feb 9, 2021

@atarax thank you for this! This looks great, I appreciate the detail on the tests + docstrings.

2 questions before merging this:

  1. Would you like to be added to the Contributors.rst?
  2. Would you be interested in adding a section to the README.rst showing people basic usage of these new methods you've added? If not , we can add an issue for someone else to add examples.
  1. Of course, happy to be part of the community :)
  2. I added examples to README.rst

@hydrosquall
Copy link
Owner

@atarax This looks great, thanks for adding the examples to the Readme! I don't see your edits on Contributors.rst, but if you include how you'd like your name / email to be included, I can add that to the release notes when this gets published to PyPi.

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.

One small doc change and this will be good to go.

README.rst Outdated
# it was reported to SEC. Set to False if you want to get data containing
# corrections
fundamentals_stmnts = CLIENT.get_fundamentals_statements('GOOGL',
startDate='2020-1-1',
Copy link
Owner

Choose a reason for hiding this comment

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

Can we edit this the startdate month/date for startDate are 0 padded like in the earlier example? Not sure if the API will handle the different number of digits correctly, so probably good to err on the safe side.

Copy link
Author

Choose a reason for hiding this comment

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

It actually can, but i changed it for consistency ;)

@kaessert kaessert force-pushed the feature/fundamentals branch from 73d92e5 to 2fa5f0c Compare February 10, 2021 09:53
@kaessert
Copy link
Author

Contributors.rst

i don't see a file with that name? oO

@hydrosquall
Copy link
Owner

hydrosquall commented Feb 10, 2021 via email

@kaessert kaessert force-pushed the feature/fundamentals branch from 2fa5f0c to a2cfbc7 Compare February 11, 2021 10:35
@kaessert
Copy link
Author

Apologies @atarax, too much context switching between projects lately: here it is :) https://github.com/hydrosquall/tiingo-python/blob/master/AUTHORS.rst

On Wed, Feb 10, 2021 at 5:18 AM atarax @.***> wrote: Contributors.rst i don't see a file with that name? oO — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#581 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACE2MMYEW23UYSCEBDHGXEDS6JMP7ANCNFSM4XGNC6FQ .

no worries, i know that kind of issues :)

@hydrosquall
Copy link
Owner

Thanks for doing this with clean git history too! Awesome work @atarax !

@hydrosquall hydrosquall reopened this Feb 12, 2021
@hydrosquall hydrosquall merged commit 6dc49a3 into hydrosquall:master Feb 12, 2021
@kaessert
Copy link
Author

Thanks for doing this with clean git history too! Awesome work @atarax !

You're welcome! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants