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

Return list[str] when passed multiple values from python #53

Merged
merged 30 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3c1e813
RED: one test changed
MusicalNinjaDad Jun 4, 2024
5ddac71
change return type of fizzbuzz
MusicalNinjaDad Jun 4, 2024
4e396de
ONE GREEN: impl IntoPy for FizzBuzzReturn
MusicalNinjaDad Jun 4, 2024
b1787fd
almost GREEN: all pyo3 tests updated and correct
MusicalNinjaDad Jun 4, 2024
ff91bfc
all pyo3 tests passing
MusicalNinjaDad Jun 4, 2024
b39797d
pytests passing
MusicalNinjaDad Jun 4, 2024
01933b8
GREEN: update docs & examples
MusicalNinjaDad Jun 4, 2024
fad959b
fmt
MusicalNinjaDad Jun 4, 2024
6970224
add overload type hints
MusicalNinjaDad Jun 4, 2024
4de884c
performance really is 2x worse with variable return types
MusicalNinjaDad Jun 5, 2024
b1c0c73
only perftest rust
MusicalNinjaDad Jun 5, 2024
e95e065
simplify FizzBuzzReturn::IntoPy
MusicalNinjaDad Jun 5, 2024
ac7387d
tidy imports
MusicalNinjaDad Jun 5, 2024
1b981de
simplify FizzBuzzReturn
MusicalNinjaDad Jun 5, 2024
7c031d4
reset to: Fix: MacOS14 CIBW bug (won't move wheel after test) (#50)
MusicalNinjaDad Jun 5, 2024
46f61e4
Improve docstring re steps
MusicalNinjaDad Jun 5, 2024
67824dd
performance test results from Union return
MusicalNinjaDad Jun 5, 2024
2853852
perftest returning a list vs union vs string (with 10M elements)
MusicalNinjaDad Jun 5, 2024
f7f1a2e
revert to returning an str
MusicalNinjaDad Jun 5, 2024
9f449f7
simplify docstring re slices
MusicalNinjaDad Jun 5, 2024
ac45a87
reimplement Union type returns
MusicalNinjaDad Jun 5, 2024
60f04a7
From FizzBuzz for Cow etc.
MusicalNinjaDad Jun 6, 2024
4fdc3d0
static cow for huge performance boosts - many tests disabled
MusicalNinjaDad Jun 6, 2024
f3094f9
reset Cow implementations ... this should be a separate piece of work
MusicalNinjaDad Jun 7, 2024
1a5d0ea
document how-to
MusicalNinjaDad Jun 7, 2024
92df97e
punctuation for legibility
MusicalNinjaDad Jun 7, 2024
71d355c
Merge remote-tracking branch 'origin/main' into MusicalNinjaDad/issue51
MusicalNinjaDad Jun 7, 2024
e0ee223
fmt
MusicalNinjaDad Jun 7, 2024
add2264
review suggestion: add test for empty list
MusicalNinjaDad Jun 7, 2024
2cc0ba1
review suggestion: documentation improvements
MusicalNinjaDad Jun 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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
28 changes: 20 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 sinlge 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,20 @@ 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 `[]`.
"""
107 changes: 81 additions & 26 deletions rust/fizzbuzzo3/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use std::ops::Neg;

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

#[derive(FromPyObject)]
Expand All @@ -25,6 +29,23 @@ impl IntoPy<Py<PyAny>> for MySlice {
}
}

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 +55,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 sinlge 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 +71,25 @@ 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.
/// Negative steps require start > stop, positive steps require stop > start; other combinations return `[]`.
#[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 +164,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 +190,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 +209,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 +228,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 +241,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 +254,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 +267,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 +285,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
Loading