Function default values

[NiklasVousten]

The rust function signature did not allow default values. Instead the user had to know these values to fill them in.

Every paramter, that has a default value, will now be wrapped inside an option. A user can this then set to None.
If this option is None, it will be excluded from the kwargs dict, and thus the default value will be used.

Furthermore, the Optional Value from typing is excluded.

I am not sure if everything was done correctly and every occurence was changed.
There might be edge cases where this will break, however on the tested examples this worked fine.

[NiklasVousten]

The pipeline is now failing with the same clippy issues as the last commit on main.

So I assume all issues on my side are fixed

[codecov]

Codecov Report

Attention: Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.

Project coverage is 66.73%. Comparing base (cd5f278) to head (149c0c7).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pyo3_bindgen_engine/src/syntax/function.rs	89.65%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
+ Coverage   66.55%   66.73%   +0.18%     
==========================================
  Files          22       22              
  Lines        3199     3223      +24     
==========================================
+ Hits         2129     2151      +22     
- Misses       1070     1072       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AndrejOrsula]

Thanks a lot for your contributions @NiklasVousten!

This is a great addition! I was wondering how to achieve something similar with builders akin to Bon, but this PR does it more directly while still being compatible with such wrappers.

I did a quick test, but I think there are some unsupported cases:

1. The updated identifier optional_* containing .is_some() check does not seem to get created/propagated in some class methods. I think getting rid of the new identifier (without the "optional_" prefix) would be ideal to resolve this issue (and directly check for p_*.is_some() where needed.

2. kwargs seem to be tricky and sometimes result in both redundant and non-compilable code. I included a specific case related to Python dataclass below

Special case with dataclass

Full source: https://github.com/Farama-Foundation/Gymnasium/blob/81b87efb9f011e975f3b646bab6b7871c522e15e/gymnasium/envs/registration.py#L79C1-L122C76

@dataclass
class EnvSpec:
    id: str
    entry_point: EnvCreator | str | None = field(default=None)

    # Environment attributes
    reward_threshold: float | None = field(default=None)
    nondeterministic: bool = field(default=False)

    # Wrappers
    max_episode_steps: int | None = field(default=None)
    order_enforce: bool = field(default=True)
    autoreset: bool = field(default=False)
    disable_env_checker: bool = field(default=False)
    apply_api_compatibility: bool = field(default=False)

    # Environment arguments
    kwargs: dict = field(default_factory=dict)

    # post-init attributes
    namespace: str | None = field(init=False)
    name: str = field(init=False)
    version: int | None = field(init=False)

    # applied wrappers
    additional_wrappers: tuple[WrapperSpec, ...] = field(default_factory=tuple)

    # Vectorized environment entry point
    vector_entry_point: VectorEnvCreator | str | None = field(default=None)

Neither of the optional_p is being used

pub fn new<'py>(
    py: ::pyo3::marker::Python<'py>,
    p_id: &str,
    p_entry_point: Option<impl ::pyo3::IntoPy<::pyo3::Py<::pyo3::types::PyAny>>>,
    p_reward_threshold: ::std::option::Option<f64>,
    p_nondeterministic: Option<bool>,
    p_max_episode_steps: ::std::option::Option<i64>,
    p_order_enforce: Option<bool>,
    p_autoreset: Option<bool>,
    p_disable_env_checker: Option<bool>,
    p_apply_api_compatibility: Option<bool>,
    p_kwargs: Option<impl ::pyo3::types::IntoPyDict>,
    p_additional_wrappers: Option<&[::pyo3::Bound<'py, ::pyo3::types::PyAny>]>,
    p_vector_entry_point: Option<
        impl ::pyo3::IntoPy<::pyo3::Py<::pyo3::types::PyAny>>,
    >,
) -> ::pyo3::PyResult<::pyo3::Bound<'py, Self>> {
    let optional_p_entry_point = p_entry_point.is_some();
    let p_entry_point = ::pyo3::IntoPy::<::pyo3::Py<::pyo3::types::PyAny>>::into_py(
        p_entry_point,
        py,
    );
    let p_entry_point = p_entry_point.bind(py);
    let optional_p_nondeterministic = p_nondeterministic.is_some();
    let optional_p_order_enforce = p_order_enforce.is_some();
    let optional_p_autoreset = p_autoreset.is_some();
    let optional_p_disable_env_checker = p_disable_env_checker.is_some();
    let optional_p_apply_api_compatibility = p_apply_api_compatibility.is_some();
    let optional_p_kwargs = p_kwargs.is_some();
    let p_kwargs = ::pyo3::types::IntoPyDict::into_py_dict_bound(p_kwargs, py);
    let optional_p_additional_wrappers = p_additional_wrappers.is_some();
    let optional_p_vector_entry_point = p_vector_entry_point.is_some();
    let p_vector_entry_point =
        ::pyo3::IntoPy::<::pyo3::Py<::pyo3::types::PyAny>>::into_py(
            p_vector_entry_point,
            py,
        );
    let p_vector_entry_point = p_vector_entry_point.bind(py);
    ::pyo3::types::PyAnyMethods::extract(&::pyo3::types::PyAnyMethods::call1(
        ::pyo3::types::PyAnyMethods::getattr(
            py.import_bound(::pyo3::intern!(py, "gymnasium.envs.registration"))?
                .as_any(),
            ::pyo3::intern!(py, "EnvSpec"),
        )?
        .as_any(),
        ::pyo3::types::PyTuple::new_bound(
            py,
            [
                ::pyo3::ToPyObject::to_object(&p_id, py),
                ::pyo3::ToPyObject::to_object(&p_entry_point, py),
                ::pyo3::ToPyObject::to_object(&p_reward_threshold, py),
                ::pyo3::ToPyObject::to_object(&p_nondeterministic, py),
                ::pyo3::ToPyObject::to_object(&p_max_episode_steps, py),
                ::pyo3::ToPyObject::to_object(&p_order_enforce, py),
                ::pyo3::ToPyObject::to_object(&p_autoreset, py),
                ::pyo3::ToPyObject::to_object(&p_disable_env_checker, py),
                ::pyo3::ToPyObject::to_object(&p_apply_api_compatibility, py),
                ::pyo3::ToPyObject::to_object(&p_kwargs, py),
                ::pyo3::ToPyObject::to_object(&p_additional_wrappers, py),
                ::pyo3::ToPyObject::to_object(&p_vector_entry_point, py),
            ],
        ),
    )?)
}

---

FYI; I am busy at the moment, so I won't be able to look much more into this for a couple of weeks.

[AndrejOrsula]

Also, the tooling/test suite in the repository is generally lacking. Maybe you already found something better, but this helped me debug specific cases (not optimal, yet better than what is included...).

1. Include the following in pyo3_bindgen_engine/src/lib.rs:

mod test;

2. Iterate with this (either inside a script or as a one-liner):

rm ./pyo3_bindgen_engine/src/test.rs
touch ./pyo3_bindgen_engine/src/test.rs
cargo run -- -m <decently_complex_python_module> -o ./pyo3_bindgen_engine/src/test.rs
cargo fmt
cargo check