DOC: Add ref docs for C generic types. by tylerjereddy · Pull Request #11689 · numpy/numpy

tylerjereddy · 2018-08-08T17:34:04Z

I tried drafting a bit of reference documentation about the @TYPE@ style generic typing used in NumPy C source since even an experienced C programmer may not have seen that before (it is not standard compliant). I don't think we spell this out anywhere in docs based on a brief discussion with Matti.

Also, because I may learn something about NumPy source from the criticism of this documentation.

rgommers · 2018-08-08T17:52:28Z

would be good to make clear that this is about .c.src files. The current sentence suggest that we have .c files that aren't C89/99 compliant.

ewmoore · 2018-08-08T18:33:54Z

The macros you're trying to document aren't expanded by anything in the numpy/core/code_generators directory. The code you're looking for is part of numpy-distutils in numpy/distutils/conv_template.py.

I think the "C standard compliant" wording here is a little negative about this. Numpy uses a custom code generator that it also ships. This isn't really all that crazy of a thing to do. Its not like doing this with C macros or just accepting that massive code duplication are either good choices.

Interesting, and I see there are more "rules" than I thought in numpy/distutils/conv_template.py. I'll try to rephrase to make it a bit less framed around C standard compliance deviations, though this should likely be mentioned at least somewhere for clarity.

Instead of "are not compliant with standard C syntax" consider something like "are written in a custom templating language that is used to generate C code"

mattip · 2018-08-08T22:18:38Z

The documentation might find a better home in doc/source/dev/gitwash, which is rendered as Working with NumPy source code. I admit that whole section could use some refactoring, the git parts can just be references to other, perhaps better, generic documentation, and we should expand the parts that are NumPy specific, like this one

tylerjereddy · 2018-08-09T18:34:02Z

Revised & expanded a bit based on initial feedback; I suppose this means that any project using numpy-distutils could theoretically use the type substitution macros.

If people feel that this piece of doc is more suitably placed elsewhere as Matti suggests perhaps they can chime in--I've not visited the page he suggests before so perhaps not entirely convinced on that git workflow page being the right home.

charris · 2018-08-17T17:38:14Z

Those are not really type variables, they are intended as part of function names. Then actual types are strings like npy_byte. It is probably better to describe the operation as string substitution.

charris · 2018-08-17T17:51:16Z

It would be good to add an example, and maybe link to the module documentation in numpy/distutils/conv_template. Curiously, I can find that module documentation online, but it doesn't seem to be included in the NumPy documentation anywhere. We should maybe add it.

charris · 2018-08-17T18:00:05Z

Maybe in the distutils section doc/source/reference/distutils.rst. I wonder if we should actually move some of those distutils files into the tools directory.

tylerjereddy · 2018-10-16T20:13:17Z

I've substantially revised this DOC PR in the following ways, based on feedback above and now having some experience with our template language:

added an example
emphasize string substitution as what is actually happening
add a link to conv_template module where there are some gory details

Other notes / thoughts here:

I just stumbled across the placeholder Template files section in the distutils reference docs--I think a reviewer of my PR here could make a compelling argument that that is where this information should reside--currently there's an XXX placeholder there; at least that's evidence that these docs are needed somewhere accessible
It may eventually be worthwhile to write a custom pygments lexer for our template files so that we get nice docs with syntax highlighting for inner loop template code blocks--could probably be templated off C lexer with just another color or two for the delimiters and @ things [same could be said for syntax highlights for common editors, but anyway many of us probably just tell the editor it is a C file in config]
- for now: WARNING: Could not lex literal_block as "C". Highlighting skipped.

eric-wieser · 2018-10-17T01:21:23Z

I think a reviewer of my PR here could make a compelling argument

I'm too lazy to do that, but I think you're right - especially if this is something that we're distributing as part of distutils, and not just using internally.

tylerjereddy · 2018-10-17T23:07:45Z

Ok, I revised again to migrate the changes to disutils docs and adjust wording accordingly.

However, this raises other issues:

how does one even navigate to the distutils docs from the reference guide?
the code block and general formatting are just not functional in the rst view on github as a reviewer may notice--my changes were designed to be compiled with the rest of the docs & not simply viewed on github; so, now I'm -0.5 on migrating to the distutils docs as I've just done -- this will have to be reformatted if that's where it is staying.

mattip · 2018-10-23T08:07:46Z

how does one even navigate to the distutils docs from the reference guide?

In #12250 I linked the guide into the rendered on-line documents

eric-wieser · 2019-01-14T07:10:06Z

+     *           TIMEDELTA#
+     * #totype = npy_byte, npy_ubyte, npy_short, npy_ushort, npy_int, npy_uint,
+     *           npy_long, npy_ulong, npy_longlong, npy_ulonglong,
+     *           npy_datetime, npy_timedelta#


It would be good to note that:

These sequences are combined essentially via zip, and iterated together rather than as a cartesian product

There is a *N syntax for repeated elements

eric-wieser · 2019-01-14T07:10:43Z

-XXX: Describe how files with extensions ``.f.src``, ``.pyf.src``,
-``.c.src``, etc. are pre-processed by the ``build_src`` command.
+NumPy Distutils preprocesses C source files (extension: :file:`.c.src`) written
+in a custom templating language to generate C code. The :c:data:`@` symbol is


Only C files? It looks like it might support other files too.

eric-wieser

I think there's more to say here, but this looks like a great start, and I see no reason not to merge it as is, leaving further improvement to the future.

eric-wieser · 2019-01-14T07:14:51Z

Looking again, it seems there's a lot of overlap here with https://github.com/numpy/numpy/blob/master/doc/source/reference/distutils.rst#conversion-of-src-files.

Can you try to merge these? Perhaps doc/DISTUTILS.rst.txt should just link to doc/source/reference/distutils.rst?

mattip · 2019-03-10T10:18:15Z

@eric-wieser this PR has an approval but then a request for refactoring. Is it still approved?

eric-wieser · 2019-03-15T05:26:00Z

Let's just put this in - I've opened a new issue about merging these docs

tylerjereddy added the 04 - Documentation label Aug 8, 2018

rgommers reviewed Aug 8, 2018

View reviewed changes

tylerjereddy force-pushed the docs_C_at_symbol branch from d1810f2 to c0c3287 Compare August 8, 2018 18:19

ewmoore reviewed Aug 8, 2018

View reviewed changes

tylerjereddy force-pushed the docs_C_at_symbol branch from c0c3287 to 18ce475 Compare August 9, 2018 18:24

charris reviewed Aug 17, 2018

View reviewed changes

mattip added the 55 - Needs work label Oct 12, 2018

tylerjereddy force-pushed the docs_C_at_symbol branch from 18ce475 to 558412e Compare October 16, 2018 20:01

tylerjereddy mentioned this pull request Oct 16, 2018

WIP: ENH: Matmul as ufunc #11133

Closed

DOC: Add ref docs for C generic types.

4d9aceb

tylerjereddy force-pushed the docs_C_at_symbol branch from 558412e to 4d9aceb Compare October 17, 2018 23:01

eric-wieser reviewed Jan 14, 2019

View reviewed changes

eric-wieser approved these changes Jan 14, 2019

View reviewed changes

eric-wieser mentioned this pull request Mar 15, 2019

DOC: Merge together DISTUTILS.rst.txt#template-files" and distutils.rst#conversion-of-src-files #13129

Closed

eric-wieser merged commit 47d9d09 into numpy:master Mar 15, 2019

tylerjereddy deleted the docs_C_at_symbol branch March 15, 2019 16:36

Conversation

tylerjereddy commented Aug 8, 2018

Uh oh!

rgommers Aug 8, 2018

Choose a reason for hiding this comment

Uh oh!

ewmoore Aug 8, 2018

Choose a reason for hiding this comment

Uh oh!

tylerjereddy Aug 8, 2018

Choose a reason for hiding this comment

Uh oh!

eric-wieser Aug 9, 2018

Choose a reason for hiding this comment

Uh oh!

mattip commented Aug 8, 2018

Uh oh!

tylerjereddy commented Aug 9, 2018

Uh oh!

charris Aug 17, 2018

Choose a reason for hiding this comment

Uh oh!

charris commented Aug 17, 2018

Uh oh!

charris commented Aug 17, 2018

Uh oh!

tylerjereddy commented Oct 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric-wieser commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tylerjereddy commented Oct 17, 2018

Uh oh!

mattip commented Oct 23, 2018

Uh oh!

eric-wieser Jan 14, 2019

Choose a reason for hiding this comment

Uh oh!

eric-wieser Jan 14, 2019

Choose a reason for hiding this comment

Uh oh!

eric-wieser left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser commented Jan 14, 2019

Uh oh!

mattip commented Mar 10, 2019

Uh oh!

eric-wieser commented Mar 15, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tylerjereddy commented Oct 16, 2018 •

edited

Loading

eric-wieser commented Oct 17, 2018 •

edited

Loading

eric-wieser left a comment •

edited

Loading