fix!: fully move all dependencies on pyo3 time types under our time feature + expose new abi3 feature for documentation

[antalsz]

I somehow missed to_python last time, despite the fact that I thought I'd tried building it.

[Shadow53]

The check job uses cargo hack and checks with and without the time feature, so this seems good to me.

That said, the previous PR (#45) also passed this check, because there was nothing checking that any of those types actually implement the traits. Some simple "it compiles" checks would be useful to make sure this doesn't happen in the future. A macro would work here too, something like:

macro_rules! types_impl_trait {
    ($trait: ident, [$($ty: ty),+$(,)?]) => {
        paste::paste! {
            [< _impl_ $trait >]<T: $trait>() {}

            fn test_impls_$trait() {
                $(
                [< _impl_ $trait >]::<$ty>();
                )+
            }
        }
    }
}

types_impl_trait {
    ToPython, [
        Py<PyDate>,
        Py<PyDateTime>,
        // etc.
    ]
}

[antalsz]

I think perhaps a better test would be to try to build this crate against a pyo3 with time disabled, which should reveal things immediately – (1) is there a good way to do that?, (2) does the explicit ToPython check there add anything in that case?

[Shadow53]

I think perhaps a better test would be to try to build this crate against a pyo3 with time disabled, which should reveal things immediately – (1) is there a good way to do that?, (2) does the explicit ToPython check there add anything in that case?

Yeah, so we've got basically a 2x2 grid of possible things (time is a stand-in for anything requiring not-abi3):

	With time	Without time
With abi3	Compile error (1)	All good 👍🏻
Without abi3	Is ToPython etc. impl'd? (2)	All good 👍🏻

1. Error either from us or from pyo3/types not found
2. The explicit ToPython (etc.) would check this and give a compile error if the trait is not implemented. Requires updating tests when adding types and/or traits.

[antalsz]

Mm, I see. This fixes a different problem than the one that caused this PR (types being referenced that didn't exist), but still a worthy one. On the other hand, though, your comment "Requires updating tests when adding types and/or traits." makes me wonder if we'll actually get much of a benefit from adding those tests. It seems to me like it's just as easy to forget to add a test as it is to forget to add an implementation, and the tests won't catch you forgetting. Some more powerful "let's scrape all of these and test them" test would work here, but I'm not convinced we get a big win.

[antalsz]

I thought this / #45 were just fixes, but rigetti/qcs-sdk-rust#475 shows that they can break things that were assuming the time types were in scope – I don't know that there's anything to do about the existing releases, but I'm upgrading this to fix!:

[Shadow53]

It seems to me like it's just as easy to forget to add a test as it is to forget to add an implementation, and the tests won't catch you forgetting. Some more powerful "let's scrape all of these and test them" test would work here, but I'm not convinced we get a big win.

Valid. I do not think that should block this being merged.