Closed cekees closed 4 years ago
@JohanMabille can you take a look at this PR. I'm thinking I'd like to go ahead and split the RANS* classes to have separate modules following the same pattern. Mainly just to prevent confusion in the future. The compile times appeared to be slightly faster on my laptop, but the difference was marginal. This is mainly just for consistency/simplicity.
Also @JohanMabille can you explain why #define FORCE_IMPORT_ARRAY is necessary?
Regarding the FORCE_IMPORT_ARRAY
token, first you should be aware of this.
To summarize, the import_array
function must be called only once in the initialization routine of the extension module. Besides, each compilation unit that does not contain the initialization routine must define NO_IMPORT_ARRAY
before including numpy headers, otherwise you end up with link errors.
Now, when you include a header from xtensore-python
, you indirectly include a header form numpy. Requiring the user to define NO_IMPORT_ARRAY
in every cpp module is a bit cumbersome, therefore we took the opposite approach: by default, NO_IMPORT_ARRAY
is defined except if you define FORCE_IMPORT_ARRAY
before including a header from xtensor_python
.
This way, extension authors only have to be careful when they write the module that contains the initialization routine. Besides, when FORCE_IMPORT_ARRAY
is not defined, the import_numpy
function does nothing, preventing accidental calls to it in other cpp files.
LGTM!
I merged master back into a testing branch I made recently. This is in response to the discussion #1139 . I don't find that this compiles significantly faster, but the design is simpler in my opinion. In this version the 2D and 3D extension modules don't depend on one another, so when you're developing on say, RANS3P2D, it doesn't force recompilation of RANS3P.
Mandatory Checklist
Please ensure that the following criteria are met:
As a general rule of thumb, try to follow PEP8 guidelines.
Description