Skip to content

Auto-generate erfa.pyx and erfa.py #3159

Closed

Description

@eteq

Right now the erfa.pyx (and erfa.py from #3141) are checked in as source code. These files are entirely auto-generated using cython_generator.py, so this seems wasteful of repository bandwidth in the event of future changes to the templates or other aspects of the erfa wrappers (incl. e..g, #3137).

Some discussion of this already has happened in #2992, #3134, and #3141 . Most relevant is an issue pointed out by @astrofrog, that this would then imply a compile-time dependency on jinja2. Presumably we could avoid this for releases by having the tarballs include the autogenerated source, just as we do right now with Cython-generated c code. I personally don't think it's a big deal to require jinja2 at compile-time, because it's already necessary for sphinx, which developers should all have to build the docs.

It also might make sense to include this on astropy_helpers in a slightly more generic form, allowing any cython_generator.py sort of usage. Or we could put it only in the core for now and later generalize if there's call for it.

cc @jwoillez @astrofrog @mdboom @mhvk @embray

Activity

  1. astrofrog commented on Nov 30, 2014

    @astrofrog
    Member

    @eteq - one could be even more generic and just have a 'pre-processing' step that could be called to generate any source before doing anything else. This could also be defined in setup_package.py, and there could be a special function name for preprocessing (e.g. preprocess()).

  2. hamogu commented on Nov 30, 2014

    @hamogu
    Member

    I wonder if it's really worth the effort to change that. The files change rarely and we are talking kb here not mb. That would be different if we did use cython_generartor.py in many places, but for now we can probably use our developer time better for other issues.

  3. mhvk commented on Nov 30, 2014

    @mhvk
    Contributor

    @eteq - if we go this route (and I do like it), one thing to check is that the output is the same with python2 and python3 (currently, python2 outputs unicode strings for the warnings, like u'OK', while python3 omits the u; see #3138).

  4. cdeil commented on Dec 1, 2014

    @cdeil
    Member

    👍 to do other code-generation in addition to Cython using jinja2 if it's useful.

  5. astrofrog commented on Dec 1, 2014

    @astrofrog
    Member

    Just FYI I'm exploring ways we can support this with astropy-helpers

  6. eteq commented on Dec 1, 2014

    @eteq
    MemberAuthor

    @astrofrog - I like the idea of a generic pre-processing/generation step (although beware the condiment effect 😉). But I'm also fine with a stopgap measure that just works with erfa, given the fact that there are PRs flying around now that could use this feature.

    @hamogu - I understand where you're coming from, but in fact in this case it is MBs... The generated erfa files are ~1MB of source code... That's why I think this is worth the effort now.

    It's also part of my endless effort to keep developer overhead low: any step that requires running some tool before a commit is dangerous, because it doesn't always happen, so the generated and templates are sometimes out-of-sync. This is error-prone even for the people that write it, and this can be a huge time sink, especially for new contributors...

  7. astrofrog commented on Dec 2, 2014

    @astrofrog
    Member

    @eteq - for an implementation of auto-generation starting from one of @jwoillez's branches, see #3170

  8. astrofrog commented on Jan 13, 2015

    @astrofrog
    Member

    This is addressed by #3166

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions