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

Add options module #97

Merged
merged 7 commits into from
Mar 28, 2021
Merged

Conversation

advaiyalad
Copy link
Contributor

@advaiyalad advaiyalad commented Mar 20, 2021

New module, options

This PR adds a new module 🎉 🥳, options! This allows you to query calls and puts (options trading stuff?).

Changes

  • Add new module options
  • Add tests for the module
  • Add docs for the module

Endpoint Params

formatted - formats the response.
lang - does nothing???
region - does nothing???

Example usage

import yahooFinance from 'yahoo-finance2';
yahooFinance.options('AMZN', { formatted: false, region: 'US', lang: 'en-US' }); // default queryOptions

Tests

I have no idea why the tests are failing on CI, they pass locally perfectly.

@gadicc Anything to change/add? Any naming conflicts (that was a slight issue when making the module) that you can spot?

@gadicc
Copy link
Owner

gadicc commented Mar 22, 2021

Awesome, @PythonCreator27, thanks for yet another contribution and your fast work on this. I'm in the middle of some international travel but will take a look as soon as I'm settled at my destination. Thanks again.

@gadicc
Copy link
Owner

gadicc commented Mar 26, 2021

Hey @PythonCreator27, finally had a chance to look at this. Once again, this is really great, and I really must commend you on checking all the boxes and making a perfect fit with module style, docs, tests, etc. Thanks for your continued contributions to the project!

I only have two small comments:

  1. [options.ts:76] { moduleName: "historical" } :)

  2. There are a few additional places where I think it would be nice to coerce to date types, e.g. expirationDates array, options: [ { expirationDate, calls: [ { expiration, lastTradeDate } ], puts: [ { expiration, lastTradeDate } ]. Think that's it, and let me know if you agree.

    It's enough to mark these as Dates in the typescript interface (and re-run yarn schema), there's interim code in-place that will figure out how to convert everything correctly (if you didn't see it already, schema-tweak combined with the validateAndCoerceTypes: yahooFinanceType code).

Re CI failure, it only occurred to me now that not everyone can see the build failure messages. I'll see if we can get relevant messages directly into GitHub. The problem was the schema check... you can check if you're running the same version of ts-json-schema-generator and typescript, sometimes some of the keys change slightly with different versions, because of a different issue I need to figure out.

Thanks again! Really great work 🙌

@codecov
Copy link

codecov bot commented Mar 26, 2021

Codecov Report

Merging #97 (903215b) into devel (ce760ab) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 903215b differs from pull request most recent head cff1c8d. Consider uploading reports for the commit cff1c8d to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##             devel       #97   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        18    +1     
  Lines          311       319    +8     
  Branches        83        84    +1     
=========================================
+ Hits           311       319    +8     
Impacted Files Coverage Δ
src/modules/options.ts 100.00% <100.00%> (ø)
src/modules/quote.ts 100.00% <0.00%> (ø)

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 ce760ab...cff1c8d. Read the comment docs.

gadicc added a commit that referenced this pull request Mar 27, 2021
* slightly smaller size without irrelevant interfaces

* fixes annoying uuid change on version bump, which was for an
  irrelevant interface anyway
@gadicc
Copy link
Owner

gadicc commented Mar 27, 2021

Looks great, @PythonCreator27, thank you! Ready to merge this when you are.

I changed the schema generation to only include interfaces from the modules directory, this excludes the anonymous interface which got a new uuid every time we upgraded ts-json-schema-generator (or typescript, don't recall). Unfortunately that created a merge conflict but I'm happy to sort that out my side.

Oh I guess we should also update the docs to reflect the Date types, but since that's a non-code change I'm happy to do that after, up to you.

Thanks again for this contribution!

@advaiyalad
Copy link
Contributor Author

@gadicc Thanks for resolving the anonymous interface issue! Other than the merge conflict, I think that this should be good!

Thanks for the amazing module!

@gadicc gadicc merged commit 7ba6294 into gadicc:devel Mar 28, 2021
gadicc pushed a commit that referenced this pull request Mar 28, 2021
# [1.10.0](v1.9.1...v1.10.0) (2021-03-28)

### Features

* **optionsModule:** Add `options` module ([#97](#97)) ([7ba6294](7ba6294))
@gadicc
Copy link
Owner

gadicc commented Mar 28, 2021

🎉 This PR is included in version 1.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gadicc
Copy link
Owner

gadicc commented Mar 28, 2021

Thanks, @PythonCreator27. Looks great! Merged and released.

Will reply to your comments on other issues tomorrow. Have an awesome one :D

@advaiyalad advaiyalad deleted the feat/add-options-module branch April 8, 2021 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants