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

Refactor geoflow action fun args #262

Closed
eblondel opened this issue May 31, 2022 · 2 comments
Closed

Refactor geoflow action fun args #262

eblondel opened this issue May 31, 2022 · 2 comments
Assignees
Labels
Milestone

Comments

@eblondel
Copy link
Collaborator

eblondel commented May 31, 2022

Relates to #261
This is the current status regarding the way actions can be interfaced:

  • global actions (embedded or custom - provided by user) have to be defined with a namedfunction interface like:
my_fun <- function(entity, config, options){}

where my_fun should be the action id.

  • local actions: for which no function has to be provided

This is highly criticizable and not really convenient for the user, for multiple reasons detailed below:

  • Not the same logic is applied for global vs. local actions. Proposal: handle a function for all actions
  • For global actions, giving a function name is useless, in particular for custom actions where we constrain the user to name the function with the id defined for the action.
  • The options are not clear, especially for testing local actions, these are the action options. This 'argument' should be removed from the function interface. The action definition should be made available. From there, options will be easily grab using action$options, or using util to develop in Provide simple way to get action option value (selected, or default available) #261 which will then avoid re-writing action options default values in the action function, but inherit them from the action definition;
  • This will open the door to test implementation of Coupling provenance/lineage with local R actions #125

Hence, any action (global embedded, global custom, local) will have to be designed within a function interface as follows:

function(action, entity, config){

   opts <- action$options
   ....
}

In addition, for global embedded actions, an attempt will be made to create a separate folder 'actions' under the 'inst' folder

@abennici @juldebar @wheintz if you have any questions, feel free. I will anyway assist you in changes where needed

@bastienird
Copy link
Contributor

Are the global custom actions not concerned by this update ? As it still mandatory to have an "options" argument in the function ? (Line 517 of geoflow::initWorkflow https://github.com/eblondel/geoflow/blob/master/R/initWorkflow.R)

@eblondel
Copy link
Collaborator Author

All actions should be concerned by this code refactoring/harmonization

@eblondel eblondel reopened this Aug 10, 2022
@eblondel eblondel reopened this May 28, 2023
@eblondel eblondel removed the ongoing label May 28, 2023
eblondel added a commit to firms-gta/geoflow-tunaatlas that referenced this issue Jun 12, 2023
eblondel added a commit to firms-gta/geoflow-tunaatlas that referenced this issue Jun 12, 2023
eblondel added a commit to firms-gta/geoflow-tunaatlas that referenced this issue Jun 12, 2023
eblondel added a commit to firms-gta/geoflow-tunaatlas that referenced this issue Jun 12, 2023
eblondel added a commit to firms-gta/geoflow-tunaatlas that referenced this issue Jun 12, 2023
@eblondel eblondel changed the title Refactor geoflow action fun handling Refactor geoflow action fun args Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants