Open scottbarnes opened 1 year ago
Hey @scottbarnes , I would like to work on this issue. Could you guide me through details please?
@mekarpeles Please assign me this issue.
Please assign me this. I want to work on it
I am willing to work on this issue. Kindly assign me this issue.
Hello, @tusharv01, @ayushman075, and @Joyee2004, if you're on the Open Library Slack, please reach out to me (@Scott Barnes) so we can coordinate on this.
@scottbarnes I haven't received the Slack invite yet, and I'm eagerly looking forward to joining the community.Once I receive the invite, I'll be sure to reach out.
@scottbarnes don't have the slack invite yet.
Hi @scottbarnes. I'm not in the open library slack but I would love to help if this issue is still open
@yivgen, thanks for your interest in this. I am beginning to think this may not be the good first issue I thought it was. However, if you watch this (poorly recorded, poorly edited) video about type hints (or are already familiar with them) and still want to work on this, please let me know: https://youtu.be/gveP5Hjm62U.
It's a bit of set up for me each time someone is interested in this issue, and so far only one person has ever made it to the point of opening a PR to add type hints, so I am fairly close to closing this issue. But, if in spite of these warnings, if you watch the video (or are familiar with type hinting) and still work on this, please let me know. :)
@scottbarnes, I've watched the video and it seems relatively straightforward. It might be tricky to figure out what to do when it comes to changing the code itself but I'd love to give it a try.
Great. I will aim to generate a patch file and get back to you later today.
@yivgen, MonkeyType, which was used to generate this stub file, continues to be less and less compatible with the current version of Python, but I was able to generate the following for openlibrary/core/helpers.py
: openlibrary.core.helpers.stub.txt
You should be able to take the suggested type hints from that text file, apply them manually to openlibrary/core/helpers.py, and then continually run mypy
and edit the file until things stop failing. Please let me know if you run into any issues or have any questions.
@scottbarnes, I added the typehints in helpers.py, could you check it please?
By the way, let me know if I can help with typehints for any other files too
@yivgen, if you are interested in doing more, I've put monkeytype.sqlite3
here. If you pip install monkeytype
, you should be able to use monkeytype list-modules
, and monkeytype stub <module name>
.
You'd want to stick within the modules prefixed with openlibrary
(save for perhaps infogami
, or possibly web
-- see more below).
If you look at the MonkeyType project page you may see monkeytype apply
and think that sounds great, and it does, except it seems to rarely work these days.
What I do run monkeytype list-modules
, then monkeytype stub <module name>
, see if anything is there, and then check the file to see if any new type hints could be added on the basis of this data.
Some files you could check to start off with, which look as if they have helpful hints in them:
monkeytype stub openlibrary.book_providers
monkeytype stub openlibrary.app
(this just has one hint, but it's a helpful one for people who are new to web.py and may be wondering what that returns).One place with almost no type hints is the infogami
submodule. It's in another project, so if you made changes to that module, the PRs would go there (and I'd review them there), but Open Library relies on it. The same is true of the web
module, which is webpy.
Ok, I'll look into book_providers
and app
and maybe some other files in the openlibrary
module. And then check the infogami
and web
submodules.
Thank you for your help :)
It would be nice to have more type hints in the code base, and MonkeyType can help with that by generating type hints that are collected at runtime.
This is an issue to keep track of work on that.
A brief example of how MonkeyType can be used on this code base: #7934
Pull requests that add type hints from MonkeyType