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

Add include_once statement. #3057

Open
nicopauss opened this issue Jul 28, 2019 · 3 comments

Comments

@nicopauss
Copy link

commented Jul 28, 2019

Currently we cannot control if a file has already been included with the include statement.
I propose to add the include_once statement to only include a file once per module.
The full path of the included file is used to check if it was already included in the module.

@scoder

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2019

Hmm, the barrier for adding a new keyword to the language is fairly high. In fact, we are trying to get away from special syntax, not add to it. What if users had a line like this in their code:
include_once = True
That seems a very reasonable piece of code, which would break with your proposed change.

Could you explain a bit more what your use case is? It's easier to understand what you want when you explain why you want it. An example would help.

@nicopauss

This comment has been minimized.

Copy link
Author

commented Aug 1, 2019

We have multiple products that all share some common code base.
We want to deliver one and only one python module for each product.
Splitting a product module is not an option because of symbol resolution in our C code base.

Here is what our project looks like:

.
├── libs/
│   ├── lib_1/
│   │   └── lib_1.pxi
│   ├── lib_2/
│   │   └── lib_2.pxi
│   ├── lib_3/
│   │   └── lib_3.pxi
│   └── lib_common/
│       ├── lib_common.c
│       ├── lib_common.h
│       └── lib_common.pxi
├── product_a/
│   └── product_a.pyx
└── product_b/
    └── product_b.pyx
######## libs/lib_common/lib_common.c ########

int lib_common_super_func_in_c(int val)
{
    return ~val;
}


######## libs/lib_common/lib_common.h ########

#ifndef IS_LIB_COMMON_H
#define IS_LIB_COMMON_H

int lib_common_super_func_in_c(int);

#endif /* IS_LIB_COMMON_H */


######## libs/lib_common/lib_common.pxi ########

#cython: language_level=3

cdef extern from "lib_common.h":
    int lib_common_super_func_in_c(int)


cdef object lib_common_super_func(object val):
    return lib_common_super_func_in_c(val)


USED_LIBS = []


cdef void add_used_lib(str lib):
    USED_LIBS.append(lib)


cdef list get_used_libs():
    return USED_LIBS


add_used_lib('lib_common')


######## libs/lib_1/lib_1.pxi ########

#cython: language_level=3


include "../lib_common/lib_common.pxi"


cdef object lib_1_super_func(object val):
    return lib_common_super_func(val + 42)


add_used_lib('lib_1')


######## libs/lib_2/lib_2.pxi ########

#cython: language_level=3


include "../lib_common/lib_common.pxi"


cdef object lib_2_super_func(object val):
    return lib_common_super_func(val - 99)


add_used_lib('lib_2')


######## libs/lib_3/lib_3.pxi ########

#cython: language_level=3


include "../lib_common/lib_common.pxi"


cdef object lib_3_super_func(object val):
    return lib_common_super_func(val * 21)


add_used_lib('lib_3')


######## product_a/product_a.pyx ########

#cython: language_level=3


include "../libs/lib_1/lib_1.pxi"
include "../libs/lib_2/lib_2.pxi"


cdef object product_a_super_func():
    return lib_1_super_func(788) + lib_2_super_func(423)


print('super funcs: {}'.format(product_a_super_func()))
print('used libs: {}'.format(get_used_libs()))


######## product_b/product_b.pyx ########

#cython: language_level=3


include "../libs/lib_1/lib_1.pxi"
include "../libs/lib_3/lib_3.pxi"


cdef object product_b_super_func():
    return lib_1_super_func(54) - lib_3_super_func(4988)


print('super funcs: {}'.format(product_b_super_func()))
print('used libs: {}'.format(get_used_libs()))

The issue is that lib_1.pxi, lib_2.pxi and lib_3.pxi all include lib_common.pxi.

So when cythonizing and compiling product_a.pyx and product_b.pyx, I get some duplicate function definitions and USED_LIBS is initialized multiple times.

I can make it work by removing include "../lib_common/lib_common.pxi" in lib_1.pxi, lib_2.pxi and lib_3.pxi and add it in product_a.pyx and product_b.pyx, but we loose the dependency logic and we have to make sure that lib_common.pxi will never be included in one of the library files. It also messes up the syntax checker for the library files.

Another solution would be to use a header guard with a compile variable:

######## libs/lib_common/lib_common.hdr.pxi ########

#cython: language_level=3


IF not IS_LIB_COMMON_HDR_PXI:
    DEF IS_LIB_COMMON_HDR_PXI = True
    include "lib_common.pxi"

And to replace include "../lib_common/lib_common.pxi" by include "../lib_common/lib_common.hdr.pxi".

The issue with this solution is that we need IS_LIB_COMMON_HDR_PXI to be defined before including lib_common.hdr.pxi.

To solve this issue we need to find a way to detect if IS_LIB_COMMON_HDR_PXI is defined or not.
It is also a nice feature to have by the way.

I see two solutions:

  1. Add a is_defined function to the selection of builtin functions available at compile-time:
IF not is_defined('IS_LIB_COMMON_HDR_PXI'):
    DEF IS_LIB_COMMON_HDR_PXI = True
    include "lib_common.pxi"
  1. Add IFDEF and IFNDEF statements:
IFNDEF IS_LIB_COMMON_HDR_PXI:
    DEF IS_LIB_COMMON_HDR_PXI = True
    include "lib_common.pxi"
@scoder

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

The issue with this solution is that we need IS_LIB_COMMON_HDR_PXI to be defined before including lib_common.hdr.pxi.

True. OTOH, Cython will give you a compile time error if you test the value and it's not defined, so these cases are safe and easy to detect. I would suggest defining the variable at the top of your main module file. I think I would also lean towards testing it inside of the included file rather than in all files that include it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.