Skip to content

Commit

Permalink
Return list[str] when passed multiple values from python (#53)
Browse files Browse the repository at this point in the history
<!-- Generated by sourcery-ai[bot]: start summary -->

## Summary by Sourcery

This pull request introduces a new return type for the `fizzbuzz`
function, allowing it to return either a single string or a list of
strings based on the input. The documentation and tests have been
updated accordingly to reflect these changes.

* **Enhancements**:
- Updated the `fizzbuzzo3.fizzbuzz` function to return a list of strings
when passed a list or range of numbers, instead of a single concatenated
string.
- Enhanced the documentation to reflect the new return types for the
`fizzbuzz` function, providing clearer examples and explanations.
- fizzbuzzo3: Introduced a new return type `FizzBuzzReturn` to handle
different return types for the `fizzbuzz` function, allowing it to
return either a single string or a list of strings based on the input.
* **Documentation**:
- Added detailed documentation on how to implement and use the new
`FizzBuzzReturn` type in `docs/dev-build.md`.
- Updated the type hints and docstrings in
`python/fizzbuzz/fizzbuzzo3.pyi` to reflect the new return types for the
`fizzbuzz` function.
* **Tests**:
- Modified existing tests to check for the new return type (list of
strings) when passing a list or range of numbers to the `fizzbuzz`
function.
- Added new test cases to ensure the `fizzbuzz` function returns an
empty list for invalid slice ranges.
* **Chores**:
- Commented out the Python performance tests in `tests/perftest.py` to
focus on Rust performance comparisons.

<!-- Generated by sourcery-ai[bot]: end summary -->
  • Loading branch information
MusicalNinjaDad authored Jun 7, 2024
1 parent 9ad7d82 commit 732d1a1
Show file tree
Hide file tree
Showing 6 changed files with 285 additions and 44 deletions.
68 changes: 66 additions & 2 deletions docs/dev-build.md
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,72 @@ Python also often provides single functions which can receive multiple significa
--8<-- "rust/fizzbuzzo3/src/lib.rs"
```

!!! warning "`Union` type returns"
If you want to create something like: `#!python def fizzbuzz(n: int | list[int]) -> str | list[str]:` [Issue pyo3/#1637](https://github.com/PyO3/pyo3/issues/1637) suggests you may be able to do something with the `IntoPy` trait but I haven't tried (yet)
!!! pyo3 "`Union` type returns: `#!python def fizzbuzz(n: int | list[int]) -> str | list[str]`"
If you would like to provide different return types for different cases:

1. Implement an `enum`, or a wrapper `struct` around an existing `enum`, that holds the different types.
1. Provide one or more conversion `From` traits to convert from the return of your core rust functions.
1. Provide a conversion [`IntoPy` trait](https://docs.rs/pyo3/latest/pyo3/conversion/trait.IntoPy.html) to convert to the relevant PyO3 types.
1. Use this new type as the return of your wrapped function.
In **`/rust/fizzbuzzo3/src/lib.rs`**:
```rust
...
struct FizzBuzzReturn(FizzBuzzAnswer);

impl From<FizzBuzzAnswer> for FizzBuzzReturn {
fn from(value: FizzBuzzAnswer) -> Self {
FizzBuzzReturn(value)
}
}

impl IntoPy<Py<PyAny>> for FizzBuzzReturn {
fn into_py(self, py: Python<'_>) -> Py<PyAny> {
match self.0 {
FizzBuzzAnswer::One(string) => string.into_py(py),
FizzBuzzAnswer::Many(list) => list.into_py(py),
}
}
}
...
#[pyfunction]
#[pyo3(name = "fizzbuzz", text_signature = "(n)")]
fn py_fizzbuzz(num: FizzBuzzable) -> PyResult<FizzBuzzReturn> {
...
```

Thanks to the comments in [Issue pyo3/#1637](https://github.com/PyO3/pyo3/issues/1637) for pointers on how to get this working.

1. Add `@overload` hints for your IDE (see [IDE type & doc hinting](#ide-type-doc-hinting)), so that it understands the relationships between input and output types:

In **`/python/fizzbuzz/fizzbuzzo3.pyi`**:
```python
...
from typing import overload

@overload
def fizzbuzz(n: int) -> str:
...

@overload
def fizzbuzz(n: list[int] | slice) -> list[str]:
...

def fizzbuzz(n):
"""
Returns the correct fizzbuzz answer for any number or list/range of numbers.
...
```

??? pyo3 "**`rust/fizzbuzzo3/src/lib.rs`** - full source"
```rust
--8<-- "rust/fizzbuzzo3/src/lib.rs"
```

??? python "**`python/fizzbuzz/fizzbuzzo3.pyi`** - full source"
```python
--8<-- "python/fizzbuzz/fizzbuzzo3.pyi"
```

## IDE type & doc hinting

Expand Down
29 changes: 21 additions & 8 deletions python/fizzbuzz/fizzbuzzo3.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,17 @@ Usage:
```
"""

def fizzbuzz(n: int | list[int] | slice) -> str:
from typing import overload

@overload
def fizzbuzz(n: int) -> str:
...

@overload
def fizzbuzz(n: list[int] | slice) -> list[str]:
...

def fizzbuzz(n):
"""
Returns the correct fizzbuzz answer for any number or list/range of numbers.
Expand All @@ -21,8 +31,8 @@ def fizzbuzz(n: int | list[int] | slice) -> str:
n: the number(s) to fizzbuzz
Returns:
A `str` with the correct fizzbuzz answer(s).
In the case of a list or range of inputs the answers will be separated by `, `
In the case of a single number: a `str` with the correct fizzbuzz answer.
In the case of a list or range of inputs: a `list` of `str` with the correct fizzbuzz answers.
Examples:
a single `int`:
Expand All @@ -37,18 +47,21 @@ def fizzbuzz(n: int | list[int] | slice) -> str:
```
from fizzbuzz.fizzbuzzo3 import fizzbuzz
>>> fizzbuzz([1,3])
'1, fizz'
['1', 'fizz']
```
a `slice` representing a range:
```
from fizzbuzz.fizzbuzzo3 import fizzbuzz
>>> fizzbuzz(slice(1,4,2))
'1, fizz'
['1', 'fizz']
>>> fizzbuzz(slice(1,4))
'1, 2, fizz'
['1', '2', 'fizz']
>>> fizzbuzz(slice(4,1,-1))
'4, fizz, 2'
['4', 'fizz', '2']
>>> fizzbuzz(slice(1,5,-1))
[]
```
Note: Slices are inclusive on the left, exclusive on the right and can contain an optional step.
Negative steps require start > stop.
Negative steps require start > stop, positive steps require stop > start; other combinations return `[]`.
A step of zero is invalid and will raise a `ValueError`.
"""
104 changes: 79 additions & 25 deletions rust/fizzbuzzo3/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::ops::Neg;

use fizzbuzz::{FizzBuzz, MultiFizzBuzz};
use fizzbuzz::{FizzBuzz, FizzBuzzAnswer, MultiFizzBuzz};
use pyo3::{exceptions::PyValueError, prelude::*, types::PySlice};
use rayon::iter::{IndexedParallelIterator, IntoParallelIterator, ParallelIterator};

Expand All @@ -25,6 +25,24 @@ impl IntoPy<Py<PyAny>> for MySlice {
}
}

/// A wrapper struct for FizzBuzzAnswer to provide a custom implementation of `IntoPy`.
struct FizzBuzzReturn(FizzBuzzAnswer);

impl From<FizzBuzzAnswer> for FizzBuzzReturn {
fn from(value: FizzBuzzAnswer) -> Self {
FizzBuzzReturn(value)
}
}

impl IntoPy<Py<PyAny>> for FizzBuzzReturn {
fn into_py(self, py: Python<'_>) -> Py<PyAny> {
match self.0 {
FizzBuzzAnswer::One(string) => string.into_py(py),
FizzBuzzAnswer::Many(list) => list.into_py(py),
}
}
}

/// Returns the correct fizzbuzz answer for any number or list/range of numbers.
///
/// This is an optimised algorithm compiled in rust. Large lists will utilise multiple CPU cores for processing.
Expand All @@ -34,8 +52,8 @@ impl IntoPy<Py<PyAny>> for MySlice {
/// n: the number(s) to fizzbuzz
///
/// Returns:
/// A `str` with the correct fizzbuzz answer(s).
/// In the case of a list or range of inputs the answers will be separated by `, `
/// In the case of a single number: a `str` with the correct fizzbuzz answer.
/// In the case of a list or range of inputs: a `list` of `str` with the correct fizzbuzz answers.
///
/// Examples:
/// a single `int`:
Expand All @@ -50,23 +68,27 @@ impl IntoPy<Py<PyAny>> for MySlice {
/// ```
/// from fizzbuzz.fizzbuzzo3 import fizzbuzz
/// >>> fizzbuzz([1,3])
/// '1, fizz'
/// ['1', 'fizz']
/// ```
/// a `slice` representing a range:
/// ```
/// from fizzbuzz.fizzbuzzo3 import fizzbuzz
/// >>> fizzbuzz(slice(1,4,2))
/// '1, fizz'
/// ['1', 'fizz']
/// >>> fizzbuzz(slice(1,4))
/// '1, 2, fizz'
/// ['1', '2', 'fizz']
/// >>> fizzbuzz(slice(4,1,-1))
/// '4, fizz, 2'
/// ['4', 'fizz', '2']
/// >>> fizzbuzz(slice(1,5,-1))
/// []
/// ```
/// Note: Slices are inclusive on the left, exclusive on the right and can contain an optional step.
/// Negative steps require start > stop.
/// Note: Slices are inclusive on the left, exclusive on the right and can contain an optional step.
/// Negative steps require start > stop, positive steps require stop > start; other combinations return `[]`.
/// A step of zero is invalid and will raise a `ValueError`.
#[pyfunction]
#[pyo3(name = "fizzbuzz", text_signature = "(n)")]
fn py_fizzbuzz(num: FizzBuzzable) -> PyResult<String> {
fn py_fizzbuzz(num: FizzBuzzable) -> PyResult<FizzBuzzReturn> {
match num {
FizzBuzzable::Int(n) => Ok(n.fizzbuzz().into()),
FizzBuzzable::Float(n) => Ok(n.fizzbuzz().into()),
Expand Down Expand Up @@ -141,8 +163,15 @@ mod tests {
#[pyo3import(py_fizzbuzzo3: from fizzbuzzo3 import fizzbuzz)]
fn test_fizzbuzz_vec() {
let input = vec![1, 2, 3, 4, 5];
let result: String = fizzbuzz!(input);
assert_eq!(result, "1, 2, fizz, 4, buzz");
let expected = vec![
"1".to_string(),
"2".to_string(),
"fizz".to_string(),
"4".to_string(),
"buzz".to_string(),
];
let result: Vec<String> = fizzbuzz!(input);
assert_eq!(result, expected);
}

#[pyo3test]
Expand All @@ -160,8 +189,15 @@ mod tests {
stop: 6,
step: Some(1),
};
let result: String = fizzbuzz!(input);
assert_eq!(result, "1, 2, fizz, 4, buzz");
let expected = vec![
"1".to_string(),
"2".to_string(),
"fizz".to_string(),
"4".to_string(),
"buzz".to_string(),
];
let result: Vec<String> = fizzbuzz!(input);
assert_eq!(result, expected);
}

#[pyo3test]
Expand All @@ -172,8 +208,15 @@ mod tests {
stop: 6,
step: None,
};
let result: String = fizzbuzz!(input);
assert_eq!(result, "1, 2, fizz, 4, buzz");
let expected = vec![
"1".to_string(),
"2".to_string(),
"fizz".to_string(),
"4".to_string(),
"buzz".to_string(),
];
let result: Vec<String> = fizzbuzz!(input);
assert_eq!(result, expected);
}

#[pyo3test]
Expand All @@ -184,8 +227,9 @@ mod tests {
stop: 6,
step: Some(2),
};
let result: String = fizzbuzz!(input);
assert_eq!(result, "1, fizz, buzz");
let expected = vec!["1".to_string(), "fizz".to_string(), "buzz".to_string()];
let result: Vec<String> = fizzbuzz!(input);
assert_eq!(result, expected);
}

#[pyo3test]
Expand All @@ -196,8 +240,9 @@ mod tests {
stop: 0,
step: Some(1),
};
let result: String = fizzbuzz!(input);
assert_eq!(result, "");
let result: Vec<String> = fizzbuzz!(input);
let expected: Vec<String> = vec![];
assert_eq!(result, expected);
}

#[pyo3test]
Expand All @@ -208,8 +253,9 @@ mod tests {
stop: 0,
step: Some(-2),
};
let result: String = fizzbuzz!(input);
assert_eq!(result, "buzz, fizz, 1");
let expected = vec!["buzz".to_string(), "fizz".to_string(), "1".to_string()];
let result: Vec<String> = fizzbuzz!(input);
assert_eq!(result, expected);
}

#[pyo3test]
Expand All @@ -220,8 +266,14 @@ mod tests {
stop: 1,
step: Some(-1),
};
let result: String = fizzbuzz!(input);
assert_eq!(result, "buzz, 4, fizz, 2");
let expected = vec![
"buzz".to_string(),
"4".to_string(),
"fizz".to_string(),
"2".to_string(),
];
let result: Vec<String> = fizzbuzz!(input);
assert_eq!(result, expected);
}

#[pyo3test]
Expand All @@ -232,8 +284,10 @@ mod tests {
stop: 0,
step: Some(-2),
};
let result: String = fizzbuzz!(input);
assert_eq!(result, "fizz, 4, 2");

let expected = vec!["fizz".to_string(), "4".to_string(), "2".to_string()];
let result: Vec<String> = fizzbuzz!(input);
assert_eq!(result, expected);
}
#[pyo3test]
#[allow(unused_macros)]
Expand Down
Loading

0 comments on commit 732d1a1

Please sign in to comment.