Add Discrete(dtype) parameter

[JDRanpariya]

Description

Add support for dtype in Discrete Space.

Fixes # (issue)
#1118

Type of change

Please delete options that are not relevant.

• Documentation only change (no code changed)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[pseudo-rnd-thoughts]

Could you move the changes then good to go

[pseudo-rnd-thoughts]

I forgot to request testing,

@pytest.mark.parametrize("dtype, sample_dtype", [
    (int, np.int64),
    (np.int64, np.int64),
    (np.int32, np.int32),
    (np.uint8, np.uint8),
])
def test_dtype(dtype, sample_dtype):
    space = Discrete(n=5, dtype=dtype, start=2)

    sample = space.sample()
    sample_mask = space.sample(mask=np.array([0, 1, 0, 0, 0], dtype=np.int8))
    print(f'{sample=}, {sample_mask=}')
    print(f'{type(sample)=}, {type(sample_mask)=}')
    assert isinstance(sample, sample_dtype), type(sample)
    assert isinstance(sample_mask, sample_dtype), type(sample_mask)


@pytest.mark.parametrize("dtype", [
    str,
    np.float32,
    np.complex64,
])
def test_dtype_error(dtype):
    with pytest.raises(ValueError, match="Invalid Discrete dtype"):
        Discrete(4, dtype=dtype)

Could you add this to tests/spaces/test_discrete.py

[JDRanpariya]

Have added the tests, thanks!

[Kallinteris-Andreas]

Also Test dtype equals none

[JDRanpariya]

Hey thanks for the check, I didn't bother to look if None was covered, it failed for None and had to raise error the way. Now I think of it, we aren't doing any checks for multi discrete dtype. I might add it or maybe create a issue as good first.

[Kallinteris-Andreas]

Yes, feel free to create a PR for multi-discrete testing next.

[pseudo-rnd-thoughts]

I just realised that we need to update the contains to work with other dtype.
@JDRanpariya Could you update contains and add to test_dtype, checking that the samples are contained within the space.
It might be worth checking if failure cases as well including int for np.int64 dtype and empty shape arrays

[JDRanpariya]

Hmm, there is no test for contains, to_jsonable and from_jsonable, would have to know the expected behavior and write tests for all three of them.

Would look into it, shortly. I would like to spend sometime looking at whole code base for a while.