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

[Enh] Add interface function with conversionFinalizer dict, and branching on diff column in the library #32

Open
mw66 opened this issue Oct 15, 2022 · 1 comment

Comments

@mw66
Copy link

mw66 commented Oct 15, 2022

As discussed elsewhere, add the issue here with the code location:

libmir/mir-algorithm#442 (comment)

You'd better allow user to plug in his own converters for his own data.

It is allowed now. Please check the conversionFinalizer.

conversionFinalizer : (
            unquotedString,
            scalar,
            columnIndex,
            columnName)

So you pass out the columnIndex and columnName to let the user branch in his function to dispatch to different columns? This looks ugly, and may incur code duplication, e.g. two different csv, with col-1, col-2 swapped:

in Python, pass in dict, 2 one-liner:

data1 = np.genfromtxt("data1.csv", ..., converters = {1: cvtr1, 2: cvtr2})
data2 = np.genfromtxt("data2.csv", ..., converters = {2: cvtr1, 1: cvtr2})

But with conversionFinalizer in D:

conversionFinalizer1 (...) {
  switch (columnIndex) {
    1: return cvtr1(unquotedString);
    2: return cvtr2(unquotedString);
  }
}

conversionFinalizer2 (...) {
  switch (columnIndex) {
    1: return cvtr2(unquotedString);
    2: return cvtr1(unquotedString);
  }
}

too verbose.

Why not use the Python dictionary format, and let the Mir do such branching in the library?

Example in D can just follow the Python api:

double cvtr1(string str) {return ...;}
double cvtr2(string str) {return ...;}

data1 = mir.genfromtxt("data1.csv", ...,  [1: cvtr1, 2: cvtr2]);  // D does not have named arg yet, let just use positional arg
data2 = mir.genfromtxt("data2.csv", ...,  [2: cvtr1, 1: cvtr2]);  // pass in D's AA.
@9il
Copy link
Member

9il commented Oct 16, 2022

We don't have plans to extend the current API but MRs are welcome.

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

No branches or pull requests

2 participants