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

Reload + Polars not working #177

Closed
Lundez opened this issue Jun 26, 2023 · 3 comments
Closed

Reload + Polars not working #177

Lundez opened this issue Jun 26, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@Lundez
Copy link
Contributor

Lundez commented Jun 26, 2023

Hi!

Polars (DataFrame) is built using Rust-bindings and this doesn't seem to work with Solara "Auto-Reload" function.

After first reload I get NameError: name 'PyDataFrame' is not defined, I believe because it's not reloading the imports correctly.
Because Polars uses rust it's using PyO3 to bind into Rust.

It's hard to debug as I don't understand Solara in-depth yet.
This works in streamlit.

How to reproduce:

import solara
import polars as pl

@solara.component
def Page():
    file, set_file = solara.use_state(None)
    solara.FileDrop(on_file=set_file, lazy=False)
    solara.Text(f"Hello World!")
    Page2(file)

@solara.component
def Page2(file):

    if file is not None:
        solara.Text(f"File")
        df = pl.read_csv(file["data"])
        solara.DataFrame(df.to_pandas())
    else:
        solara.Text("No file, please upload one.")

Change a line and try to load a CSV again and it'll fail.. :/
Works on first try after starting server

Additionally I get a warning Warning: polars binary missing! which seems to suggest Rust bindings failing.

@maartenbreddels
Copy link
Contributor

Hi Lundez,

thanks for opening the issue. This is related to #148
I hope to take a look at this soon, or at least provide a workaround.

Thank you,

Maarten

@maartenbreddels maartenbreddels added the bug Something isn't working label Jun 26, 2023
@Lundez
Copy link
Contributor Author

Lundez commented Jun 27, 2023

@maartenbreddels if I add polars to the ignored reloaded imports (reload.py), e.g.

reload_modules = {
            k for k in set(sys.modules) - set(self.ignore_modules) if not (k.startswith("solara.server") or k.startswith("anyio") or k.startswith("pandas") or k.startswith("polars"))
        }

it works!

@Lundez
Copy link
Contributor Author

Lundez commented Jun 28, 2023

Just to keep things both trackable (GitHub) and in more private discussions (Discord) I share our discussion here.

Lundez

@maartenbreddels i found that if we don't reload polars everything works as expected. I see pandas is excluded in the list (along with more libs), as such I did the same with polars. Posted this in issue.

I went further and was thinking, what modules do we wish to reload? Perhaps we can only reload modules from our cwd (i.e. sys.path[0])? Why would we want to reload dependencies?

Marten

@Londet i have that way of reloading now locally, because I also ran into issues. I think that should be the default for reloading from now on. Great we reach similar conclusions 🙂 I'd like to compare to streamlit as well btw, because looking at their code, I think they also reload everything.

maartenbreddels added a commit that referenced this issue Jun 29, 2023
* test: enable reload tests

* fix: do not reload modules that are not under the app directory

Fixes #177
Fixes #148

* clean up code

* __file__ can be None

* call clear from the test code

* move to subdir so we do not reload the test modules
maartenbreddels added a commit that referenced this issue Jul 20, 2023
* fix: autorouting should pick up modules that only define routes

Instead of having a Page component, we should also just allow a user
to define a set of routes.
This is what we need for the /apps/scrolling example.

* fix: hot reload was too aggresive (#179)

* test: enable reload tests

* fix: do not reload modules that are not under the app directory

Fixes #177
Fixes #148

* clean up code

* __file__ can be None

* call clear from the test code

* move to subdir so we do not reload the test modules

* also catch AttributeError when app is None during unitesting

* fix: solara patches should have no effect outside a solara context

* pass cls

* fix: ignore typing

* pass cls

* fix: ignore typing

---------

Co-authored-by: Maarten A. Breddels <maartenbreddels@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants