Add option base model

[jeffweng8]

Checklist

• [x] Added tests for changed code.
• [x] Updated documentation for changed code.
• [x] I've added a news fragment of my changes with the name, "{ISSUE_NUM}.{feature|bugfix|doc|removal|misc}""

Related Issue

https://github.com/robinhood-unofficial/pyrh/issues/201

Description

• Updated several of the endpoints to be able to support options
• Streamline instrument getter functions
• Add option model class

[adithyabsk]

Just ping me when you want me to take a look again. Also, if you're having trouble with the linter, take a look at the docs to see how to set up pre-commit which should automatically fix most of the issues.

[jeffweng8]

Hey @adithyabsk

I am getting pyrh/common.py:5: error: Cannot find implementation or library stub for module named 'yarl' pyrh/common.py:5: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports pyrh/models/base.py:6: error: Cannot find implementation or library stub for module named 'marshmallow' pyrh/models/base.py:78: error: Class cannot subclass 'Schema' (has type 'Any') pyrh/models/portfolio.py:5: error: Cannot find implementation or library stub for module named 'marshmallow' pyrh/models/option.py:5: error: Cannot find implementation or library stub for module named 'marshmallow' pyrh/models/oauth.py:6: error: Cannot find implementation or library stub for module named 'marshmallow' pyrh/models/oauth.py:46: error: unused 'type: ignore' comment pyrh/models/sessionmanager.py:11: error: Cannot find implementation or library stub for module named 'marshmallow' pyrh/models/sessionmanager.py:14: error: Cannot find implementation or library stub for module named 'yarl' pyrh/models/sessionmanager.py:497: error: unused 'type: ignore' comment Found 10 errors in 6 files (checked 13 source files) when I try to run the linter locally. Do you know how to resolve this issue?

[adithyabsk]

@jeffweng8 I think you might not have the packages installed locally, perhaps? (or at least not in a place mypy can find them) pre-commit is installed in a separate virtualenv so that it doesn't depend on your env but mypy needs those installations to properly static check our code. Can you try manually running mypy pyrh from the project base directory?

[jeffweng8]

Ah that makes sense, I just installed yarl and marshmallow in the virtual environment and that fixed the issue. Thanks!

[adithyabsk]

@jeffweng8 you likely want to install all of our dev dependencies either using pip install . or using poetry install -vvv (poetry is reccomended)

[jeffweng8]

Hmm I am getting this error when I try to run the poetry install command:

Traceback (most recent call last): File "/Users/jeff/anaconda3/bin/poetry", line 7, in from poetry.console import main ModuleNotFoundError: No module named 'poetry'

Any idea where that is coming from? I ran pip install . and it successfully ran.

Also, when I try to run pytest, I get the following: ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...] pytest: error: unrecognized arguments: --xdoc --cov=pyrh --cov-config=setup.cfg --cov-report=term --cov-report=html inifile: /Users/jeff/Documents/pyrh/setup.cfg rootdir: /Users/jeff/Documents/pyrh

Is this related to the above issue?

[jeffweng8]

Also should I close this PR and make a new PR where I squash my commits?

[adithyabsk]

Nope, no worries. I can squash when I merge @jeffweng8

[adithyabsk]

Yep @jeffweng8, so I would actually suggest you not use your anaconda version of python when installing poetry since anaconda comes with a bunch of packages pre-installed.

$ which python

Should tell you the info you need there. I would use your system version of python (pre-installed on macs or brew version of python or better yet use pyenv to manage your python version)

You do this by looking at your PATH, which you likely modified to be able to use anaconda (putting it first in front of other directories your system would want to search)

[adithyabsk]

Additionally if you use poetry it will automatically manage your virtual environment so you can just do poetry run [CMD] and access you venv easily. (E.g. poetry run pytest

[adithyabsk]

It is also important to note how you install poetry, you shouldn’t install it just using pip, take a look at their docs and there should be a one line command that your run which uses your defined python version in your path to set it up.

[adithyabsk]

@jeffweng8 just to keep you in the loop, I'm starting to work on re-factoring the endpoints part of the project. That's the next big task. I've started the a draft PR #226. Take a look to get a sense of the direction that I'm heading there.

[jeffweng8]

Cool thanks for the update!

[jeffweng8]

Hey @adithyabsk I uninstalled my conda and reinstalled python 3.7 but now I am running into this issue when I try to run pre-commit run -a:

An unexpected error has occurred: CalledProcessError: command: ('/usr/local/Cellar/pre-commit/2.2.0/libexec/bin/python3.7', '-mvirtualenv', '/Users/jeff/.cache/pre-commit/repogtmyd8el/py_env-python3.6', '-p', 'python3.6') return code: 1 expected return code: 0 stdout: RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.6'

stderr: (none)

I tried deleting the cache and creating a new virtual environment but I keep running this issue. I am thinking that this is may be due to my previous install of python 3 in conda which was version 3.6. Do you have any idea how to fix this?

[adithyabsk]

@jeffweng8 Sure, I think you might first need to run pre-commit uninstall (hopefully that isn't bound to an old version of python). Also, pre-commit clean will remove the pre-commit files from its venv. Then you should be able to follow the steps from the docs to re-install pre-commit.

Basically you need to just nuke everything related to pre-commit and try to set it up again because it is confused since the python version it was using to set things up no longer exists. If none of the above work, then I would try to brew uninstall pre-commit and brew install pre-commit which should fix the links.

One general thing I might do is to is try to clean up the python versions installed on your machine. Once you have a good handle on all the python versions installed on your machine, you might want to look into pyenv if you want multiple python versions on your machine.

[jeffweng8]

Hmm I tried re-installing pre-commit, as well as python 3.7 as well as using pyenv, but I am still running into the same error. I looked into this file: https://github.com/robinhood-unofficial/pyrh/blob/master/.pre-commit-config.yaml and I see this:

• repo: https://github.com/ambv/black rev: 19.10b0 hooks:
  • id: black language_version: python3.6

Do I need to update the python version in the yaml file? Or should I not touch that file

[adithyabsk]

That's a great point.

[adithyabsk]

That's likely it. Since I have pyenv, installed and multiple versions of python setup on my machine it didn't complain since pre-commit was able to find it. Let me patch it on master and you can rebase.

[adithyabsk]

Try now.

[jeffweng8]

works now

[jeffweng8]

I noticed this in the documentation for the BaseModel class:

If a passed parameter is a nested dictionary, then it is created with the UnknownModel class.

Why is this? does this mean an attribute cannot point to a dictionary?

[adithyabsk]

@jeffweng8 The idea is that everything in the API wrapper should be a Python object and not a dictionary. What the UnknownModel allows us to do is to play nice with Robinhood API changes where users can deal with / contribute upstream any breakages from RH's end.

[adithyabsk]

This way, nothing breaks if the returned schema changes but we still have "turtles all the way down"

[jeffweng8]

I don't quite understand. Isn't a dictionary a python object?

[adithyabsk]

@jeffweng8 It is but there is a difference between interacting with the API as:

model['some_attr']['another_attr']
# vs
model.some_attr.another_attr

[adithyabsk]

That's the whole reason we are using marshmallow which allows us to seamlessly convert between JSON <--> Python custom classes. Additionally, custom classes will allow us to transact and perform business logic easily as well. (i.e. build orders and place orders using an instrument object for example)

[jeffweng8]

Hmm so from the response from the options instruments endpoint, there is a field called min_ticks that would be something like this: { "above_tick": "0.05", "below_tick": "0.01", "cutoff_price": "3.00" } and this is loaded as type UnknownModel. What exactly is marshmallow doing here?

[adithyabsk]

You should make a nested schema for that. Call it something like MinTicksSchema. Take a look at oauth.py file which nests Challenge within OAuth if it exists.

[jeffweng8]

Got it

[adithyabsk]

@jeffweng8 #227 Here's some more sample code on how to work with Marshmallow that might be helpful

[jeffweng8]

@adithyabsk Could you merge your changes in base schema from the other PR? That way I can use it to create the option chain models

[adithyabsk]

I don't have tests for that code, yet. I can merge it and note to myself that I need to add tests.

[adithyabsk]

Also, for some reason, I'm seeing diffs on this branch when I try to CR for changes that I made on my earlier branch. I think you might have needed to rebase rather than merge. (GitHub gets easily confused on PRs if you merge rather than rebase)

[jeffweng8]

Hmm, I did a git rebase, but there were some conflicts that I had to resolve, maybe I incorrectly resolved the conflicts?

[adithyabsk]

@jeffweng8 Actually, I stand corrected. That would do it since when you resolved the conflicts the commit hash would have changed making GitHub think those changes were introduced in this PR. I can manage when CR'ing I'll just manually reference master.

[adithyabsk]

Okay it's merged: https://github.com/robinhood-unofficial/pyrh/pull/227

[jeffweng8]

Cool thanks!

[adithyabsk]

@jeffweng8 the merge just fixed the diff:

If you want to simulate a two-dot diff in a pull request and see a comparison between the most recent versions of each branch, you can merge the base branch into your topic branch, which updates the last common ancestor between your branches.

[jeffweng8]

Hey @adithyabsk

I just pushed the skeleton for the chain model. I need to fix the session manager for get_chain, but I'm not too sure what exactly that is supposed to be

[adithyabsk]

@jeffweng8 I'll be quite busy for the next few weeks but I'll try to take a look this weekend. Also let me know when this PR is in a good state for CR and I'll try to get that in.

Also, to give you a sense of my next steps, I know I need to step up a better way to run tests on this project especially with Robinhood constantly updating their API. To that end, I'm likely going to try and set up vcr.py and pytest-vcr so that writing tests is as easy as calling the desired endpoints and caching the results. It will be somewhat difficult to initially set it up to work with oauth but once that is done it should be smooth sailing from there.

[adithyabsk]

I need to fix the session manager for get_chain

Also, what specifically is broken?

[jeffweng8]

No rush, I was just unsure what exactly a session manager is supposed to be. Is it just getting the results from a page? If so should I create a default one since most of them should just be grabbing it from the "results" key?

[adithyabsk]

SessionManager allows you to authenticate with the Robinhood API. Take a look at how I structured the Portfolio modules. I made a PortfolioManager which the main Robinhood class inherits from. The idea is that all "managers" should inherit from it and as a result get access to the managed forms of the get and post methods which allow you to hit the endpoints that are restricted to only logged in users.

Eventually, we should be able to mark specific methods as not requiring log-in which should work without the authentication flow.

[jeffweng8]

Hi @adithyabsk, I think this PR is pretty much ready. Could you help me see why this is failing some of the checks? When I run pytest locally, everything passes, but it appears that this is happening for some of the checks here:

`Creating virtualenv pyrh-6aazBjZI-py3.7 in /Users/runner/.virtualenvs [FileNotFoundError] [Errno 2] No such file or directory

[error]Process completed with exit code 1.`

[adithyabsk]

@jeffweng8 that's related to this: https://github.com/robinhood-unofficial/pyrh/issues/233

If you notice, your build passed on Linux. Thanks for the PR, I'll try to do a proper CR in the next week.

[jeffweng8]

Hey @adithyabsk have you had a chance to look at the changes yet?

[jeffweng8]

Hi @adithyabsk , Are you still planning on working on or maintaining this project?

[adithyabsk]

@jeffweng8 Sorry for the long delay in response, I'll look at this in the coming week.

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

Closing this pull request automatically because it has not had any activity since it has been marked as stale. If you think it is still relevant and should be addressed, feel free to open a new one.