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

Port to Python3 #34

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Saumya-Mishra9129
Copy link
Member

@Saumya-Mishra9129 Saumya-Mishra9129 commented May 13, 2020

  • Port to Python3
  • Port from sgmllib
  • RuntimeError: generator raised StopIteration

Done with first one but the last one is still not fixed.
@quozl @pro-panda Review.

@rhl-bthr
Copy link

Reviewed the draft, thanks

@Saumya-Mishra9129 Saumya-Mishra9129 marked this pull request as ready for review May 27, 2020 18:07
@Saumya-Mishra9129
Copy link
Member Author

Screenshot from 2020-06-06 15-18-46
Getting this error after above changes.

@srevinsaju
Copy link
Member

@Saumya-Mishra9129 try replacing all raise StopIteration to return

@chimosky
Copy link
Member

chimosky commented Jun 7, 2020

A StopIteration exception is raised by the built-in next or an iterator's __next__ method when they're no further items produced by the iterator.

As it's coming from BeautifulSoup see #29 as fixing it might also fix this.

@srevinsaju
Copy link
Member

A StopIteration exception is raised by the built-in next or an iterator's next method when they're no further items produced by the iterator.

There has been a new PEP regarding this after the release of Python 3.7+ afaik, it works differently from that version onwards. Python3.6 and before would not face the problem.


As mentioned here #29 (comment), how about depending on distro specific bs4 package?

@Saumya-Mishra9129
Copy link
Member Author

As mentioned here #29 (comment), how about depending on distro specific bs4 package?

Yeah we should use distro specific package. It is most common these days and a good practice.

There has been a new PEP regarding this after the release of Python 3.7+ afaik, it works differently from that version onwards. Python3.6 and before would not face the problem.

I agree with you . Since Python 3.7, all StopIteration exceptions raised inside a generator are transformed into RuntimeError (see PEP-0479 and from StackOverflow).

@quozl
Copy link
Contributor

quozl commented Jun 29, 2020

What's the status here? It looks like work in progress, but not yet fixed a StopIteration exception yet, and the exception is a normal part of next.

@Saumya-Mishra9129
Copy link
Member Author

What's the status here? It looks like work in progress, but not yet fixed a StopIteration exception yet, and the exception is a normal part of next.

Thanks for review. I am working with distro specific packages of BeautifulSoup , I haven't pushed the work here yet. I don't have any idea about how to solve StopIteration , as it is in BeautifulSoup. , so I chose to work on #33 first.

@quozl
Copy link
Contributor

quozl commented Jun 29, 2020

Thanks. BeautifulSoup is somewhat outdated, and Python 3 now has parsers onboard. Why would you continue to use BeautifulSoup?

@srevinsaju
Copy link
Member

Yes, I agree with @quozl , python3's html.parser is promising. External dependencies always make people reluctant to use an activity. Maybe by using the built in parser, it would be very portable.

@Saumya-Mishra9129
Copy link
Member Author

Thanks. BeautifulSoup is somewhat outdated, and Python 3 now has parsers onboard. Why would you continue to use BeautifulSoup?

Yes, I agree with @quozl , python3's html.parser is promising. External dependencies always make people reluctant to use an activity. Maybe by using the built in parser, it would be very portable.

I even agree with both of you. I didn't think about using built in parsers and eventually land up on solving #29 . Thanks for the suggestion. I will now move forward with built-in parsers.

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.

5 participants