separate lib, ipyvizzu, streamlit modules - one package

[veghdev]

todo:

• [ ] remove code duplication from unit tests (after it remove --disable=duplicate-code from Makefile)
• [ ] remove 'no cover' unit test comments from init.py - test these lines with mock (if it is possible)
• [ ] update docs

[veghdev]

@nyirog I started another draft for separating lib, python, jupyter, streamlit, etc. modules.

In this solution we'll use one package that contains a module (chartlib) which is an abstract chart class, and from that an inherited python, jupyter and streamlit charts.

In the other draft I separated the modules into different packages because if we add jupyter, streamlit, etc as a dependency, everyone needs to install all of them. But if someone wants to use ipyvizzu for example in streamlit, most likely streamlit already installed and propably not want to install it via ipyvizzu. So I removes IPython as a dependency.

In used to help the import, I add a small script into ipyvizzu/init.py that tries to find the environment where ipyvizzu used.

[nyirog]

@nyirog I started another draft for separating lib, python, jupyter, streamlit, etc. modules.

In this solution we'll use one package that contains a module (chartlib) which is an abstract chart class, and from that an inherited python, jupyter and streamlit charts.

It seems good, that only that user have to install IPython who imports from ipyvizzu.jupyter.

In the other draft I separated the modules into different packages because if we add jupyter, streamlit, etc as a dependency, everyone needs to install all of them. But if someone wants to use ipyvizzu for example in streamlit, most likely streamlit already installed and propably not want to install it via ipyvizzu. So I removes IPython as a dependency.

I don't know how data scientists work, but it is a common practice to create new virtualenv for every new project, therefore I would not guess about the existence of IPython or streamlit, instead I would list them as extras_require in the setup.py. So the user who want to use jupyter notebook would install ipyvizzu[jupyter] and the other user who want to use streamlit would install ipyvizzu[streamlit]:

setup(
    ...,
    extras_require={
      "jupyter": ["IPython"],
      "streamlit": ["streamlit"],
    }
)

In used to help the import, I add a small script into ipyvizzu/init.py that tries to find the environment where ipyvizzu used.

I think it is a good practice to catch the ImportError of IPython or streamlit on module level:

from .python.chart import Chart as PythonChart

try:
    from .jupyter.chart import Chart as JupyterChart
except ImportError:
    JupyterChart = None

try:
    from .streamlit.chart import Chart as StreamlitChart
except ImportError:
    StreamlitChart = None


def get_chart():
    return JupyterChart or StreamlitChart or PythonChart

Ps.: Sorry for the late reply, but I don't have even zero but "minus" free time.

[veghdev]

I think it is a good practice to catch the ImportError of IPython or streamlit on module level:

@nyirog Thank, it's an easier way. I think we should add runtime checking in addition to import checking and it should be placed in init.py (do not raise error if someone imports StreamlitChart with the full path). What do you think?

from .python.chart import Chart as PythonChart

try:
    from .jupyter.chart import Chart as JupyterChart
    import IPython  # type: ignore

    if not IPython.get_ipython():  # pragma: no cover
        raise ImportError("JupyterChart")
except ImportError as e:  # pragma: no cover
    JupyterChart = None  # type: ignore

try:
    from .streamlit.chart import Chart as StreamlitChart
    import streamlit as st

    if not st.scriptrunner.script_run_context.get_script_run_ctx():  # pragma: no cover
        raise ImportError("StreamlitChart")
except ImportError:  # pragma: no cover
    StreamlitChart = None  # type: ignore


def get_chart():
    """A method for returning the appropriate Chart for the environment."""

    return JupyterChart or StreamlitChart or PythonChart


Chart = get_chart()

[nyirog]

I think it is a good practice to catch the ImportError of IPython or streamlit on module level:

@nyirog Thank, it's an easier way. I think we should add runtime checking in addition to import checking and it should be placed in init.py (do not raise error if someone imports StreamlitChart with the full path). What do you think?

from .python.chart import Chart as PythonChart

try:
    from .jupyter.chart import Chart as JupyterChart
    import IPython  # type: ignore

    if not IPython.get_ipython():  # pragma: no cover
        raise ImportError("JupyterChart")
except ImportError as e:  # pragma: no cover
    JupyterChart = None  # type: ignore

try:
    from .streamlit.chart import Chart as StreamlitChart
    import streamlit as st

    if not st.scriptrunner.script_run_context.get_script_run_ctx():  # pragma: no cover
        raise ImportError("StreamlitChart")
except ImportError:  # pragma: no cover
    StreamlitChart = None  # type: ignore


def get_chart():
    """A method for returning the appropriate Chart for the environment."""

    return JupyterChart or StreamlitChart or PythonChart


Chart = get_chart()

Yes, we should hide import error from the user and report error only when the user asked for a specific Chart (StreamlitChart or JupyterChar) but the required packages are not installed. We can raise a runtime error in that case. The runtime error should be risen from a constructor (__init__) or other factory class method.

p.s.: In the example above the Chart = get_chart() will be evaluated at import time so I'm not sure what do you mean about runtime check. I guess you refer to an extra code which is not included in the example.