jina-ai / jina

☁️ Build multimodal AI applications with cloud-native stack
https://docs.jina.ai
Apache License 2.0
20.99k stars 2.22k forks source link

Avoid manual setting of attributes from parameters in all executors #910

Closed bhavsarpratik closed 3 years ago

bhavsarpratik commented 4 years ago

Describe the feature We have to set attributes manually from the parameters in all classes

Your proposal Add store_attr method to BaseExecutor inspired by fastcore as explained in this notebook

┆Issue is synchronized with this Jira Task by Unito

hanxiao commented 4 years ago

i would prefer using native python>= 3.7 data class if we want to enable this sugary feature. Without native support, many IDE/lint tools can not recognize those attributes anymore.

Besides, in Jina Executor is nearly always used with a YAML spec, not directly be called in Python. The one who uses this sugary feature would be the executor developers.

hanxiao commented 4 years ago

ref: https://docs.python.org/3.7/library/dataclasses.html

JoanFM commented 4 years ago

Hey @bhavsarpratik ,

I can't see what benefit is extracted from seeing the notebook, would you mind elaborating a little more?

JoanFM commented 4 years ago

Ah ok, I see now from fastcore.

hanxiao commented 4 years ago

@bhavsarpratik also beware that, using dataclass in Jina is not out-of-the-box mainly because BaseExecutor and its meta class ExecutorType is implemented in a non-trivial way. Especially on the __init__, class reading, and YAML IO-friendly. Our implementation may conflict with the functionality that dataclass provides. I would look at the source code of dataclass first before adding to BaseExecutor.

I had this idea of refactoring BaseExecutor using dataclass in the very early days on Jina 0.1.x, but that was put to a low-priority and didn't follow up. Some challenges if I remembered correctly:

I later considered it not worth it, as it mainly provides a sugary Python API, which is not really the main interface for Jina users. As I said, most of the Jina users will interact with the executor via the YAML interface.

hanxiao commented 4 years ago

In general, dataclass is a nice-to-have feature as it does make our code more Pythonic and state-of-the-art. But it has to be taken very carefully and also understand the target users who enjoy this benefit is mainly the executor developers

bhavsarpratik commented 4 years ago

I agree dataclass has its limitations. I suggested this experimental PR for developers to write shorter pythonic code for future executors.

hanxiao commented 4 years ago

for reference, https://github.com/jina-ai/jina-hub/pull/400 points out a pretty smart and non-intrusive way of using dataclass

jina-bot commented 3 years ago

This issue is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 4 days