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

FIX: coroutines can have a return statement #542

Merged
merged 5 commits into from
Apr 20, 2024

Conversation

tacaswell
Copy link
Contributor

The value returned by the coroutine is carried on the StopIteration Exception that is raised when exhausted.

This is a follow up to #145 which allows documenting. This came up in the discussions then (#47 (comment)), but I missed that it was forbidden!

The value returned by the coroutine is carried on the `StopIteration` Exception
that is raised when exhausted.
@tacaswell
Copy link
Contributor Author

I do not understand why the 3.8 test works on main (as pip install sphinx<=7.2 fails on py38 for me locally....)

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes themselves LGTM, thanks @tacaswell

Re: the sphinx pinning - running locally I get 5 sphinx.errors.ExtensionError related to overwrite_pygments_css. Is this what motivated the pinning for you @tacaswell ? It seems this is a known issue (sphinx-doc/sphinx#12299) which should be fixed in 7.3.7 - +1 from me for temporary pins until the sphinx 7.3 smoke clears.

@tacaswell
Copy link
Contributor Author

Yes, I was seeing a handful of errors from sphinx (I did not dig too deep) but given the amount of other smoke from sphinx 7.3 (and that tests passed on main with 7.2.6) I took a guess and pinned back.

@rossbar
Copy link
Contributor

rossbar commented Apr 19, 2024

@jarrodmillman this one is ready to go but I'm not sure what's going on with the CI status. I tried to merge but the only thing it allows is "enabling" merging once these jobs finish. I'm happy to go in and fixup the CI to disallow this but I don't to meddle if it might be violating some policy. Personally I'd be in favor of removing the "Required" tag on these jobs, especially since the dependencies can get quite complicated for numpydoc's support windows.

@tacaswell tacaswell closed this Apr 19, 2024
@tacaswell tacaswell reopened this Apr 19, 2024
@tacaswell
Copy link
Contributor Author

oh, I see what happened...the version pinning changed the job name which changed to miss the rules...

@jarrodmillman jarrodmillman merged commit ef0a8b5 into numpy:main Apr 20, 2024
48 of 49 checks passed
@stefanv stefanv added this to the 1.8.0 milestone Apr 20, 2024
@jarrodmillman jarrodmillman mentioned this pull request Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants