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 a crate doc link for IterNextOutput #3129

Merged
merged 3 commits into from
May 2, 2023

Conversation

cfour2
Copy link
Contributor

@cfour2 cfour2 commented May 1, 2023

@DataTriny
Copy link
Contributor

DataTriny commented May 1, 2023

First time contributor has agreed to the new licensing scheme.

@adamreichold
Copy link
Member

I am not sure if the code example in the linked module-level documentation is really sufficient. I guess the best documentation on this we currently have is https://pyo3.rs/v0.18.3/class/protocols#iterable-objects but that is not part of the inline documentation.

Would you be up to replacing the link by a dedicated example on how to use IterNextOutput

You can find the old example referenced here at https://github.com/PyO3/pyo3/blob/f4724dc1aa522d57f91401ef4842d8ed1e9bd546/src/class/iter.rs

@adamreichold adamreichold added the CI-skip-changelog Skip checking changelog entry label May 1, 2023
@cfour2
Copy link
Contributor Author

cfour2 commented May 1, 2023

Hi @adamreichold ,

The old example seems moved here:
https://github.com/PyO3/pyo3/blob/main/pytests/src/pyclasses.rs#L15-L36

Do you prefer replacing the link by a dedicated example or just pointing it to the pytests code above?

@cfour2 cfour2 force-pushed the cfour2-fix-doc-IterNextOutput-link branch from 030603f to 8b94929 Compare May 1, 2023 14:42
@adamreichold
Copy link
Member

An inline example (even if it starts as a copy of the test) would definitely be helpful as it can be viewed without leaving the inline documentation (which for example is also available offline).

@adamreichold
Copy link
Member

I fear the CI is also broken independently of this change request. I will try to fix this separately...

@cfour2
Copy link
Contributor Author

cfour2 commented May 1, 2023

The example is copied together with some comments. It should look like this here:

use pyo3::prelude::*;
use pyo3::iter::IterNextOutput;

#[pyclass]
struct PyClassIter {
    count: usize,
}

#[pymethods]
impl PyClassIter {
    #[new]
    pub fn new() -> Self {
        PyClassIter { count: 0 }
    }

    fn __next__(&mut self) -> IterNextOutput<usize, &'static str> {
        if self.count < 5 {
            self.count += 1;
            // Given an instance `counter`, First five `next(counter)` calls yield 1, 2, 3, 4, 5.
            IterNextOutput::Yield(self.count)
        } else {
            // At the sixth time, we get a `StopIteration` with `'Ended'`.
            //     try:
            //         next(counter)
            //     except StopIteration as e:
            //         assert e.value == 'Ended'
            IterNextOutput::Return("Ended")
        }
    }
}

And good luck with the CI...

src/pyclass.rs Outdated Show resolved Hide resolved
@adamreichold
Copy link
Member

And good luck with the CI...

The CI should be fixed now. I you remove the link, consent to the license change, fix the formatting and rebase onto the main branch, this should be good to go.

src/pyclass.rs Outdated Show resolved Hide resolved
@cfour2 cfour2 force-pushed the cfour2-fix-doc-IterNextOutput-link branch from 4b83a32 to f2b4761 Compare May 2, 2023 07:07
* Remove the (eventually) stale ref link;
* Typo: couter -> counter;
* Fix the formatting with `cargo fmt`;
@cfour2 cfour2 force-pushed the cfour2-fix-doc-IterNextOutput-link branch from f2b4761 to 04f129f Compare May 2, 2023 07:16
@cfour2
Copy link
Contributor Author

cfour2 commented May 2, 2023

Thanks for the help and patience! All issues mentioned above are now resolved.

  • doc
    • Remove the (eventually) stale link;
    • Fix the typo: couter -> counter;
    • cargo fmt;
  • consent to the license change;
  • rebase onto the main branch;
  • CI;

@adamreichold
Copy link
Member

Thank you for going the extra mile and helping us to improve the documentation!

bors r+

@bors
Copy link
Contributor

bors bot commented May 2, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-skip-changelog Skip checking changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants