StarlightSearch / EmbedAnything

A minimalist yet highly performant, lightweight, lightning fast, multisource, multimodal and local embedding solution, built in rust.
Apache License 2.0
304 stars 25 forks source link

Rust library requires type `F` even when no adapter is passed #93

Open boswelja opened 1 week ago

boswelja commented 1 week ago

Describe the bug

Embed functions like embed_website require a generic type F to be specified. This type is the type for the callback adapter. However, adapter is optional and we can therefore pass None. This leads to a type resolution problem though, since F is required.

It is possible to work around this, but it's not super clean - I ended up with

let result = embed_webpage::<Box<dyn Fn(Vec<EmbedData>)>>(
    "https://developer.android.com/jetpack/androidx/compose-roadmap".into(),
    &embedder,
    None,
    None
).await

Expected behavior

I should be able to call embed_webpage without a type, especially if I'm not passing an adapter.

Screenshots

image

akshayballal95 commented 1 week ago

You are right. This is indeed a known problem and a fix is appreciated. But I don't understand how the workaround fixes the problem. Isn't it just moving the type definition. We still need to give a type. If you see in the example web_embed.rs, I am doing something similar:

    let embed_data = embed_webpage(
        url,
        &embeder,
        Some(&embed_config),
        None::<fn(Vec<EmbedData>)>,
    )
    .await
    .unwrap()
    .unwrap();

But feel free to open a PR if you figure out a cleaner solution.

boswelja commented 1 week ago

Yeah the workaround I mentioned was how I got around the type error, not really how to avoid the issue in the library 😅

boswelja commented 1 week ago

Off the back of #92, I created embed_html and I think I've found a function signature that doesn't require generics as such See https://github.com/StarlightSearch/EmbedAnything/pull/94/files#diff-f64796ccc388f201986ea7eaaf07bbefbd8ac1a56bb995fb505da80879872a39R237

akshayballal95 commented 1 week ago

Thanks for pointing me in the right direction. It doesn't work as it is, but making the following change removes the requirement for a type definition in the function call. I need to see if it works well with PyO3, though.

-  adapter: Option<impl FnOnce(Vec<EmbedData>)>
+ adapter: Option<Box<dyn FnOnce(Vec<EmbedData>)>>