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

Hide executor from users #300

Closed
Tracked by #302
RobbeSneyders opened this issue Jul 18, 2023 · 0 comments
Closed
Tracked by #302

Hide executor from users #300

RobbeSneyders opened this issue Jul 18, 2023 · 0 comments
Assignees

Comments

@RobbeSneyders
Copy link
Member

Each component now has the following section at the bottom of their src/main.py file:

if __name__ == "__main__":
    executor = PandasTransformExecutor.from_args()
    executor.execute(MyComponent)

Since you always need to match the right executor class with the implemented component class, this is something fondant can do for the users.

Ideally we can remove the whole if __name__ == "__main__" block by calling a fondant module / command as entrypoint instead of the src/main.py module.

@RobbeSneyders RobbeSneyders converted this from a draft issue Jul 18, 2023
RobbeSneyders added a commit that referenced this issue Jul 18, 2023
This PR follows up on the PoC presented in #268

---

Fixes #257 

It splits the implementation and execution of components, this has some
advantages:

- Pandas components can use `__init__` instead of setup, which is
probably more familiar to users
- Other components can use `__init__` as well instead of receiving all
arguments to their transform or equivalent method, aligning
implementation of different component types
- Component implementation and execution should be easier to test
separately

I borrowed the executor terminology from KfP.

---

Fixes #203 

Since I had to update all the components, I also switched some of them
to subclass `PandasTransformComponent` instead of
`DaskTransformComponent`.

---

These changes open some opportunities for additional improvements, but I
propose to tackle those as separate PRs as this PR is already quite huge
due to all the changes to the components.

- [ ] #300
- [ ] #301
satishjasthi pushed a commit to satishjasthi/fondant that referenced this issue Jul 21, 2023
This PR follows up on the PoC presented in ml6team#268

---

Fixes ml6team#257 

It splits the implementation and execution of components, this has some
advantages:

- Pandas components can use `__init__` instead of setup, which is
probably more familiar to users
- Other components can use `__init__` as well instead of receiving all
arguments to their transform or equivalent method, aligning
implementation of different component types
- Component implementation and execution should be easier to test
separately

I borrowed the executor terminology from KfP.

---

Fixes ml6team#203 

Since I had to update all the components, I also switched some of them
to subclass `PandasTransformComponent` instead of
`DaskTransformComponent`.

---

These changes open some opportunities for additional improvements, but I
propose to tackle those as separate PRs as this PR is already quite huge
due to all the changes to the components.

- [ ] ml6team#300
- [ ] ml6team#301
@PhilippeMoussalli PhilippeMoussalli moved this from Ready for development to Validation in Fondant development Aug 16, 2023
@PhilippeMoussalli PhilippeMoussalli self-assigned this Aug 16, 2023
RobbeSneyders pushed a commit that referenced this issue Aug 22, 2023
Draft PR that addresses this #300 

The new interface will be changed from this:

```python
if __name__ == "__main__":
    executor = PandasTransformExecutor.from_args()
    executor.execute(MyComponent)
```

to this: 

```python
if __name__ == "__main__":
    component_runner = ComponentRunner(MyComponent)
    component_runner.run()
```

I'v only changed one example in the tests to showcase how it would look
like. All feedback is welcome
@RobbeSneyders RobbeSneyders moved this from Validation to Done in Fondant development Aug 28, 2023
Hakimovich99 pushed a commit that referenced this issue Oct 16, 2023
This PR follows up on the PoC presented in #268

---

Fixes #257 

It splits the implementation and execution of components, this has some
advantages:

- Pandas components can use `__init__` instead of setup, which is
probably more familiar to users
- Other components can use `__init__` as well instead of receiving all
arguments to their transform or equivalent method, aligning
implementation of different component types
- Component implementation and execution should be easier to test
separately

I borrowed the executor terminology from KfP.

---

Fixes #203 

Since I had to update all the components, I also switched some of them
to subclass `PandasTransformComponent` instead of
`DaskTransformComponent`.

---

These changes open some opportunities for additional improvements, but I
propose to tackle those as separate PRs as this PR is already quite huge
due to all the changes to the components.

- [ ] #300
- [ ] #301
Hakimovich99 pushed a commit that referenced this issue Oct 16, 2023
Draft PR that addresses this #300 

The new interface will be changed from this:

```python
if __name__ == "__main__":
    executor = PandasTransformExecutor.from_args()
    executor.execute(MyComponent)
```

to this: 

```python
if __name__ == "__main__":
    component_runner = ComponentRunner(MyComponent)
    component_runner.run()
```

I'v only changed one example in the tests to showcase how it would look
like. All feedback is welcome
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants