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

Added image_url of the first listing image to the output table #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

williamy2k
Copy link

Hello, as discussed I added retrieving the primary property image. This is my first Github pull request so apologies if I got something wrong, happy to amend.

@williamy2k williamy2k mentioned this pull request Sep 5, 2022
@toby-p
Copy link
Owner

toby-p commented Apr 4, 2023

Apologies for the delay!

This seems like a good idea but I think first I'd like to understand if it slows down the requests by a significant time? If so it would be better to make it optional (as with the get_floorplans parameter).

Also, I think I'd lean towards scraping all the image urls as a list if possible as it seems like that would have a lot more use cases. Not sure how much more effort that would be...

@des1redState
Copy link

For what it's worth, I pulled this commit into my own branch and didn't notice any measurable performance impact (it's just another xpath on the pre-scraped HTML).

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.

3 participants