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

An object should be able to raise an error, if it doesn't have a verb implementation #54

Open
machow opened this issue May 17, 2019 · 3 comments
Labels
core:siu .epic High level collection of issues, which enable something useful. type:research

Comments

@machow
Copy link
Owner

machow commented May 17, 2019

Note: if implemented, this change seems impactful enough that an ADR should be written for it

For example, in the example below I create a LazyTbl for mtcars data. However, since semi_join is currently not implemented in SQL, calling semi_join(tbl_mtcars, tbl_mtcars, {"cyl": "cyl"}) results in a Pipeable.

Ideally, an object should be able to declare that by default, if it does not have a method to dispatch for a verb, that it wants to produce an error.

Options

  1. Register globally
  2. Register on each verb
  3. Search for custom method (preferred choice)

For option (3), siu could search for a (class?) method named _siu_dispatch_default, and if it exists, call it.

Impact

Overall, I think this will be a very important behavior. It brings stability to pipes, which provide very clean syntax, but can fail in confusing / unintuitive ways. It uses a similar approach to jupyter notebooks, so hopefully will not be too surprising.

Example

from sqlalchemy import create_engine
from siuba.data import mtcars
import pandas as pd

engine = create_engine('sqlite:///:memory:', echo=False)

# note that mtcars is a pandas DataFrame
mtcars.to_sql('mtcars', engine)

from siuba import semi_join
from siuba.sql import LazyTbl, show_query, collect
tbl_mtcars = LazyTbl(engine, 'mtcars')

semi_join(tbl_mtcars, tbl_mtcars, {"cyl": "cyl"})
@machow
Copy link
Owner Author

machow commented Sep 15, 2019

Another option for siuba classes, like LazyTbl, is that they could subclass something that when dispatched raises an error

@machow
Copy link
Owner Author

machow commented Nov 7, 2020

A nice way to solve this would be to subclass ABCMeta

from abc import ABCMeta

class SiuTable(metaclass = ABCMeta): 
    pass

# in siuba.dply.verbs.py
import pandas as pd
SiuTable.register(pd.DataFrame)

# in siuba.sql.verbs
SiuTable.register(LazyTbl)

Then, the default for singledispatch2 could be to dispatch on SiuTable and raise an error...

@machow
Copy link
Owner Author

machow commented Jan 5, 2022

@machow machow added the .epic High level collection of issues, which enable something useful. label Jan 5, 2022
@machow machow added this to siuba Jan 6, 2022
@machow machow moved this to Backlog in siuba Jan 20, 2022
@machow machow added this to the ops and data refactor milestone Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core:siu .epic High level collection of issues, which enable something useful. type:research
Projects
Status: Backlog
Development

No branches or pull requests

1 participant