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

get_futures() method works again after removing some class variables #65

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

maneum1
Copy link
Contributor

@maneum1 maneum1 commented Jul 6, 2023

Looks like following variables

  • streamer_symbol,
  • is_tradeable,
  • future_product

are not anymore provided by a
GET /instruments/futures
request from tastytrade

This is the error I get when using the current released db33a07

# results = symbol_search(s, 'ES')
fut = Future.get_futures(session=s)
print(fut[0])
---------------------------------------------------------------------------
ValidationError                           Traceback (most recent call last)
Cell In[4], line 2
      1 # results = symbol_search(s, 'ES')
----> 2 fut = Future.get_futures(session=s)
      3 print(fut[0])

File ~\python\venvs\env311\Lib\site-packages\tastytrade\instruments.py:666, in Future.get_futures(cls, session, symbols, product_codes)
    662 validate_response(response)
    664 data = response.json()['data']['items']
--> 666 return [cls(**entry) for entry in data]

File ~\python\venvs\env311\Lib\site-packages\tastytrade\instruments.py:666, in <listcomp>(.0)
    662 validate_response(response)
    664 data = response.json()['data']['items']
--> 666 return [cls(**entry) for entry in data]

File ~\python\venvs\env311\Lib\site-packages\pydantic\main.py:150, in BaseModel.__init__(__pydantic_self__, **data)
    148 # `__tracebackhide__` tells pytest and some other tools to omit this function from tracebacks
    149 __tracebackhide__ = True
--> 150 __pydantic_self__.__pydantic_validator__.validate_python(data, self_instance=__pydantic_self__)

ValidationError: 3 validation errors for Future
streamer-symbol
  Field required [type=missing, input_value={'symbol': '/6JU2', 'prod...2', 'symbol': '/6JZ2'}]}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.1.2/v/missing
is-tradeable
  Field required [type=missing, input_value={'symbol': '/6JU2', 'prod...2', 'symbol': '/6JZ2'}]}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.1.2/v/missing
future-product
  Field required [type=missing, input_value={'symbol': '/6JU2', 'prod...2', 'symbol': '/6JZ2'}]}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.1.2/v/missing

after commenting out these 3 Future parameters the get_futures() function works again

...
class Future(TradeableTastytradeJsonDataclass):
    """
    Dataclass that represents a Tastytrade future object. Contains information about
    the future and methods to fetch futures for symbol(s).
    """
...
    # streamer_symbol: str
    back_month_first_calendar_symbol: bool
    # is_tradeable: bool
    # future_product: 'FutureProduct'
...

…ct) since they are not anymore provided by tasty trade
@Graeme22
Copy link
Contributor

Graeme22 commented Jul 6, 2023

Hey! Thanks for the contribution. So it's not that streamer_symbol or the other keys is never present; rather, they may or may not be present. So the correct fix here would be to make them Optional and give a default value of None, not comment out the fields. Would you be willing to do that?

@maneum1
Copy link
Contributor Author

maneum1 commented Jul 7, 2023

Hi Graeme, good thought. they are documented on the Tastytrade API documentation, even though these vars are not returned when fetching Futures. I uncommented them and assigned 'None' to them to make them optional.

This is the first time I made a pull request, is there a way to link the update to this one or do I need to start a new pull request from my latest commit?

Huhh, interesting I just see that my latest commit is already shown above, Cool

@Graeme22
Copy link
Contributor

Graeme22 commented Jul 7, 2023

Haha yup, you just commit to the same branch.

So this will fail the typing tests because None is not a bool nor a string. It needs to be Optional[bool] = None like contract_size. Also, all Optional values should be moved to the bottom of the class, after all non-Optional values.

@maneum1
Copy link
Contributor Author

maneum1 commented Jul 7, 2023

hups hehe, Still learning here, ... thanks for the advise to correctly implement this. I hope I got it now right. Please check again

I thought when a variable has a default value assigned in python that this makes it optional. Obviously not entirely correct. ;-)

@Graeme22
Copy link
Contributor

Graeme22 commented Jul 7, 2023

I thought when a variable has a default value assigned in python that this makes it optional. Obviously not entirely correct. ;-)

This looks good! Actually Python doesn't have strong typing, so you're sorta correct, however the pydantic library which this project uses enforces the types.

@Graeme22 Graeme22 merged commit 95d5008 into tastyware:master Jul 7, 2023
1 check passed
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