Skip to content

BLD: should we use tempita for cython templating #13399

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

Closed
jreback opened this issue Jun 8, 2016 · 11 comments
Closed

BLD: should we use tempita for cython templating #13399

jreback opened this issue Jun 8, 2016 · 11 comments
Labels
Build Library building on various platforms

Comments

@jreback
Copy link
Contributor

jreback commented Jun 8, 2016

see here

might replace some of the manual code paths in generate_code.py

@jreback jreback added the Build Library building on various platforms label Jun 8, 2016
@jreback
Copy link
Contributor Author

jreback commented Jun 8, 2016

@gfyoung can you comment on your experience

@gfyoung
Copy link
Member

gfyoung commented Jun 8, 2016

+1 from me - it's currently used in scipy and numpy and I in fact put it to good use in one of my PR's against numpy. Also, since tempita comes with Cython (Cython.Tempita --> no extra build dependency), if it's possible to use in order to cut down on manual code (which it seems based on a quick skim of the code in pandas/src/generate_code.py), certainly!

@sinhrks
Copy link
Member

sinhrks commented Jun 8, 2016

Quite interesting. I'm just adding new cython templates to support more sparse dtypes, and it can be much simpler using it.

@jreback
Copy link
Contributor Author

jreback commented Jun 9, 2016

@gfyoung can u post a link to the PR where u r using this?

@gfyoung
Copy link
Member

gfyoung commented Jun 9, 2016

Link to PR here. Key file to examine is randint_helpers.pxi.in and the templating code here. The key part is getting the template right, as once you do, filling it in is really easy.

Useful discussion points regarding Tempita in my PR start here, as there are links to other libraries that use Tempita as well.

@jreback
Copy link
Contributor Author

jreback commented Jun 9, 2016

thanks @gfyoung

note that I have found fused types pretty useful as well. here is an example of a numeric type which is used in window.pyx & some of the algos.pyx. These are pretty easy to define and work with; and somewhat remove the need for templating.

@gfyoung
Copy link
Member

gfyoung commented Jun 9, 2016

@jreback : yes, I remember you mentioning that in that same PR <a href=https://github.com/numpy/numpy/pull/6938#discussion_r64143615">here. I guess not all maintainers are a fan of it though, and it did take some getting used to as well when I first attempted it.

@jreback
Copy link
Contributor Author

jreback commented Jun 9, 2016

right they are in newer cython (though our min 0.19.1 certainly has it)

@gfyoung
Copy link
Member

gfyoung commented Jun 9, 2016

Well different repository, different rules 😄 - fused_types may not be as applicable in this case though given that there are some things in the template to generate_code.py that aren't fuse-type-able FLOABW.

@jreback
Copy link
Contributor Author

jreback commented Jun 9, 2016

@gfyoung yeah, though if I could use a fused type I think we should. Just easier to read/maintain. They seem to act very much like 'real' types, with a tiny exception. You need to this to avoid warnings when compiling.

cdef numeric val

if numeric in cython.floating and val == val:
     # do something if we are not-NaN

@shoyer
Copy link
Member

shoyer commented Jun 9, 2016

I agree that fused types are nice, but in my experience they end up falling short. Tempita looks pretty nice, better than our ad-hoc current system.

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

Successfully merging a pull request may close this issue.

4 participants