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

[medAbstractProcess] Need more 'setParameter' methods #402

Open
mjuhoor opened this issue Jun 6, 2018 · 4 comments
Open

[medAbstractProcess] Need more 'setParameter' methods #402

mjuhoor opened this issue Jun 6, 2018 · 4 comments

Comments

@mjuhoor
Copy link

mjuhoor commented Jun 6, 2018

Some processes require more appropriate parameters than simple 'int' and 'double', etc.
It is very frustrating to try to work around this limitation.
For example, some processes might need to compute something on an array in a mesh.
We could simply pass the array name as parameter to the process.
Instead, we are forced to pass 2 integers: the field data type (cell, point or field data)
and its index in the 'array of arrays' .

In pipelines this is problematic as we might not have access to this information, and its unintuitive.
Thus this requires to implement "helper" methods, that find the array for us and then call the process
with the correct parameters.

Could be related to #370

tl;dr: Need setParameter(QString, [...]), setParameter(void*, [...])

@fcollot
Copy link

fcollot commented Jun 6, 2018

If we are to add more overloads to setParameter, it should be done in dtkAbstractProcess, not medAbstractProcess, or it will cause confusion amongst developers not familiar with how overload and override resolution works in C++.

@msermesant
Copy link

what is the status in DTK 1.0? (the one in medInria 3.0)
anything changed related to this?

@fcollot
Copy link

fcollot commented Jun 6, 2018

I checked a while back and it hasn't changed.

fcollot pushed a commit that referenced this issue Oct 2, 2019
Ok
[music3] change the background color - kept in screenshots
@mathildemerle
Copy link

(btw, it's planned to be changed for medInria 4)

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

No branches or pull requests

4 participants