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

Change the way object types are tested in DisplayType to remove excessive requirements #113

Closed
thomass-dev opened this issue Jul 29, 2024 · 4 comments

Comments

@thomass-dev
Copy link
Collaborator

thomass-dev commented Jul 29, 2024

These requirements are heavy and only presents to test object type.
They must not be present in prod requirements if they are not really used.

altair
matplotlib.figure
numpy
pandas
polars

For instance, change the way object types are tested:

TYPE_TO_DISPLAY_TYPE = {
    bool: DisplayType.BOOLEAN,
    datetime.date: DisplayType.DATE,
    datetime.datetime: DisplayType.DATETIME,
    float: DisplayType.NUMBER,
    int: DisplayType.INTEGER,
    list: DisplayType.ARRAY,
    str: DisplayType.MARKDOWN,
}

with suppress(ImportError):
    import altair
    TYPE_TO_DISPLAY_TYPE[altair.vegalite.v5.api.Chart] = DisplayType.VEGA
@augustebaum augustebaum changed the title Change the way object types are tested in DispayType to remove excessive requirements Change the way object types are tested in DisplayType to remove excessive requirements Aug 9, 2024
@rouk1
Copy link
Contributor

rouk1 commented Aug 12, 2024

This is an early optimization.

@augustebaum augustebaum removed their assignment Sep 2, 2024
@augustebaum
Copy link
Contributor

augustebaum commented Sep 16, 2024

This issue's wording is obsolete after #303, but the issue of testing an object's type without having to import all the libraries for which we offer auto-detection is still current. We have been experimenting with a string-based approach, which is simple, if dirty:

def isinstance_(o: object, type_: str):
    return type_ in [t.__module__ + '.' + t.__name__ for t in o.__class__.__mro__]

o = alt.Chart()
assert isinstance_(o, "altair.vegalite.v5.schema.core.TopLevelSpec")

@tuscland
Copy link
Member

Does this affect dependencies needed to pip install skore?

@thomass-dev
Copy link
Collaborator Author

Does this affect dependencies needed to pip install skore?

Yes. skore doesn't need anymore pandas etc.

If an user want to use the PandasDataFrameItem by passing a pandas.DataFrame object to skore, we suppose that pandas is already available in its own environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants