Skip to content

cython: Add options for specifying header and imports#619

Merged
emilio merged 1 commit intomozilla:masterfrom
petrochenkov:cythonopt
Nov 25, 2020
Merged

cython: Add options for specifying header and imports#619
emilio merged 1 commit intomozilla:masterfrom
petrochenkov:cythonopt

Conversation

@petrochenkov
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov commented Nov 17, 2020

C header .h and Cython .pxd are generated as a pair, so it's important to be able to generate them from a single config file.
Two new options in this PR allow to do it without hacks.
One option allows to "link" the .pxd to .h, and another one allows to add some imports.

(I don't plan to add any more Cython-specific options in the near future, these two are the ones with highest return-on-investment.)

# `from module cimport name1, name2` declarations added in the same place
# where you'd get includes in C.
[cython.cimports]
module = ["name1", "name2"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine though I think this could pretty much use after_includes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If imports are added to after_includes, then you'd either have two separate configs for C and Cython (which is not desirable), or use

#if 0
from module cimport name1, name2
#endif

emitted into both .h and .pxd (which is not pretty).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

@emilio emilio merged commit b0aae44 into mozilla:master Nov 25, 2020
@petrochenkov
Copy link
Copy Markdown
Contributor Author

@emilio
Apologies, ouput from this feature is non-reproducible due to HashMap, this caused the CI failure on master.
I fixed it in #622.

(Some other options using HashMaps can potentially cause similar issues too.)

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

Successfully merging this pull request may close these issues.

2 participants