| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Patch looks good.
It's a little alarming how many of these there are. It's almost tempting just to remove all the broken ones, since I imagine no one has used them in a decade, and the other argument order works fine...
Sorry, something went wrong.
|
@eric-wieser If its useful, I ran into the error because I needed to use the (int* DIM1, int* DIM2, double** ARGOUTVIEWM_ARRAY2) typemap in a project: the issue I originally submitted is a simple working example based on that project. If this typemap were removed, I would need to reformat quite a few of the function declarations in the project. However, I'm not sure if this is a common occurrence. If no one has submitted the issue for a decade, I suppose its used very rarely. Another thought: if all the versions of the ARGOUTVIEW typemap are used commonly, users may be interested in changing to ARGOUTVIEWM if they run into memory leak issues (that was the case for me). If all of the versions of the ARGOUTVIEWM typemaps are available, it is very easy for the user to change their ARGOUTVIEW typemap to a ARGOUTVIEWM typemap by simply adding a letter to their .i file. |
Sorry, something went wrong.
|
It would be nice to have tests for swig. We have tests for the random C-API using example programs in random, we have tests for f2py. Could we add some tests based off of those examples? If it is difficult, we should merge this PR as a band-aid and encourage some capable swig people to add new tests (hint, hint) |
Sorry, something went wrong.
|
@mattip I could add a test similar to Array in numpy/tools/swig/test to the same folder. What do you think? |
Sorry, something went wrong.
|
I don't think those tests are actually run. You can test that locally by running python runtests.py, it should fail before this PR and pass afterward. |
Sorry, something went wrong.
|
@mattip Yeah you're right. I checked the build log, and runtests never even enters the swig directory. To create these tests there would obviously need to be a call to swig in order to generate the wrapper C code needed to compile into a Python package. I'm not familiar with NumPy's build system, and unfortunately don't feel I have the time to dedicate at the moment to become familiar enough to learn it (currently in the dissertation writing phase of my PhD, so I'm pretty busy at the moment). Therefore, I'd like to go with your original suggestion and merge this PR as a band-aid and encourage some capable swig people to add new tests. Please let me know if there is anything I need to do on my end to make this happen. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Closes #17398