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

CLN/COMPAT: Remove raise StopIteration syntax in SparseArray.__iter__ #11622

Merged
merged 1 commit into from Nov 18, 2015

Conversation

Projects
None yet
2 participants
@kawochen
Contributor

kawochen commented Nov 17, 2015

closes #11108

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 17, 2015

Contributor

why is this bad? e.g. if you are iterating with a generator with works just fine

Contributor

jreback commented Nov 17, 2015

why is this bad? e.g. if you are iterating with a generator with works just fine

@kawochen

This comment has been minimized.

Show comment
Hide comment
@kawochen

kawochen Nov 17, 2015

Contributor

this causes a PendingDeprecationWarning in 3.5 and will be deprecated in 3.7 (PEP479)
this syntax doesn't cause a problem here, but it could if combined with a context manager, where the StopIteration exception will be caught in __exit__() and potentially causes the context manager to take a different path (depending on the presence of try-except constructs in the context manager)

Contributor

kawochen commented Nov 17, 2015

this causes a PendingDeprecationWarning in 3.5 and will be deprecated in 3.7 (PEP479)
this syntax doesn't cause a problem here, but it could if combined with a context manager, where the StopIteration exception will be caught in __exit__() and potentially causes the context manager to take a different path (depending on the presence of try-except constructs in the context manager)

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 17, 2015

Contributor

is this an API change though?

Contributor

jreback commented Nov 17, 2015

is this an API change though?

@jreback jreback added the Compat label Nov 17, 2015

@kawochen

This comment has been minimized.

Show comment
Hide comment
@kawochen

kawochen Nov 17, 2015

Contributor

No. Using return (or just leaving it out, since there is no more code) and raise StopIteration are the same in this function (except for the warnings). They are different only when there is something inside the generator that could interfere with exception propagation.

Contributor

kawochen commented Nov 17, 2015

No. Using return (or just leaving it out, since there is no more code) and raise StopIteration are the same in this function (except for the warnings). They are different only when there is something inside the generator that could interfere with exception propagation.

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 17, 2015

Contributor

ok seems reasonable. add a whatsnew note.

Contributor

jreback commented Nov 17, 2015

ok seems reasonable. add a whatsnew note.

@jreback jreback added this to the 0.17.1 milestone Nov 17, 2015

@kawochen

This comment has been minimized.

Show comment
Hide comment
@kawochen

kawochen Nov 17, 2015

Contributor

which section?

Contributor

kawochen commented Nov 17, 2015

which section?

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 17, 2015

Contributor

I guess API changes; this is mainly to have a record of this.

Contributor

jreback commented Nov 17, 2015

I guess API changes; this is mainly to have a record of this.

@kawochen

This comment has been minimized.

Show comment
Hide comment
@kawochen

kawochen Nov 18, 2015

Contributor

green

Contributor

kawochen commented Nov 18, 2015

green

jreback added a commit that referenced this pull request Nov 18, 2015

Merge pull request #11622 from kawochen/COMPAT-11108
CLN/COMPAT: Remove raise StopIteration syntax in SparseArray.__iter__

@jreback jreback merged commit 89f46a5 into pandas-dev:master Nov 18, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 18, 2015

Contributor

thanks!

Contributor

jreback commented Nov 18, 2015

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment