Scraping prescription drug prices from Rx site using the prescription drug name and zipcode #5959
Conversation
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
|
I have fixed all the requests. And added tests as well |
|
Fixed the issues and tested with black. This is my first contribution so please bear with me. |
|
#5960 is driving our automated testing a bit crazy. |
| request_url: str = f'https://www.wellrx.com/prescriptions/{drug_name}/{zip_code}/?freshSearch=true' | ||
| response: Response = get(request_url) |
There was a problem hiding this comment.
Let's put type hints on function parameters and function return types but we do not need them everywhere. Both Python and mypy are capable of figuring out that a string literal is a string. ;-) Overuse slows down both the writer and reader of code.
| request_url: str = f'https://www.wellrx.com/prescriptions/{drug_name}/{zip_code}/?freshSearch=true' | |
| response: Response = get(request_url) | |
| request_url = f'https://www.wellrx.com/prescriptions/{drug_name}/{zip_code}/?freshSearch=true' | |
| response = get(request_url) |
| response: Response = get(request_url) | ||
|
|
||
| # Is the status code ok? | ||
| if response.status_code == 200: |
There was a problem hiding this comment.
Let's use response.raise_for_status() instead to let the caller know what the problem is.
https://docs.python-requests.org/en/master/api/#requests.Response.raise_for_status
There was a problem hiding this comment.
added response.raise_for_status() and changed the code accordingly.
| # Get price of the drug. | ||
| price: str = grid.find( | ||
| "span", {"p", "price price-large"}).text | ||
| formatted_price: float = format_price(price) |
There was a problem hiding this comment.
What is the reason to get rid of the $ and the two digits to the right of the decimal point? Are we going to do math (add subtract, multiply, divide) on these numbers? If not, let's not modify the formatting.
There was a problem hiding this comment.
removed the conversion as we were not using those values
| else: | ||
| return None | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
There was a problem hiding this comment.
changed except Exception as e: with except (HTTPError, exceptions.RequestException, ValueError):. This was a new learning for me.
| drug_name: str = input("Enter drug Name:\n") | ||
| zip_code: str = input("Enter zip code:\n") |
There was a problem hiding this comment.
See README.md for advise for leading and/or trailing spaces in input().
There was a problem hiding this comment.
Went through readme and codebase and added the input as
drug_name = input("Enter drug name: ").strip()
zip_code = input("Enter zip code: ").strip()
ghost
added
awaiting changes
ghost
added
awaiting reviews
|
Thank you for the code review. These are really good points that i've missed. I'll go ahead with the requested changes. |
|
@cclauss Please check my commit. I've added all the requested changes. |
saptarshi1996
changed the title
Describe your change:
Checklist:
Fixes: #{$ISSUE_NO}.