Subject: [Instant-dev] What remains before we can release Instant 1.0 ?

From: Kent Andre <[email-masked]>
Date: Tue, 13 Jan 2009 09:59:30 +0100

As always, improved documentation and better tests. 
Anything else ? 

Kent

---

From: "Martin Sandve Alnæs" <[email-masked]>
Date: Tue, 13 Jan 2009 10:10:26 +0100

There is some small stuff in the TODO file.
And there are some TODO and FIXMEs in the code.
This is probably overlapping.
In particular, there are some old arguments
that may be removed or must be explained.

TODO:
- Fix arguments not used in setup.py

- Remove unused csrcs stuff.

- Explain arrays argument.

- Use object files argument.

- Improve documentation

- Add tests for all variants of cache mechanisms available now

- Clean up imports, don't use import *

martinal@martinal-xps:~/dev/fenics/instant/src/instant$ egrep FIXME *
build.py:          FIXME: Describe this correctly. Tests pass arrays
of arrays of strings.
build.py:        instant_error("FIXME: setup.py doesn't use the C sources.")
codegeneration.py:      - additional_definitions (FIXME: comment)
codegeneration.py:      - additional_declarations (FIXME: comment)
codegeneration.py:      - arrays (FIXME: comment)
codegeneration.py:    #instant_warning("FIXME: Not using csrcs in
write_setupfile().")
codegeneration.py:    arrays = [] # FIXME: Example input here
martinal@martinal-xps:~/dev/fenics/instant/src/instant$ egrep TODO *
build.py:# TODO: Import only the official interface
build.py:        # TODO: Error instead? Indicates race condition
build.py:        - If you want to compile the files yourself. TODO:
Not yet supported.
cache.py:# TODO: We could make this an argument, but it's used
indirectly several places so take care.
__init__.py:# TODO: Import only the official interface
inlining.py:    # TODO: Something more robust? Regexp?
inlining.py:    # TODO: This isn't very general!
inlining.py:    # TODO: This isn't very general!
inlining.py:    # TODO: Is the second and third path here necessary?
inlining.py:    # TODO: Is the second and third path here necessary?

Martin

On Tue, Jan 13, 2009 at 9:59 AM, Kent Andre <[email-masked]> wrote:
> _______________________________________________
> Instant-dev mailing list
> [email-masked]
> http://fenics.org/mailman/listinfo/instant-dev
>

---

From: Ilmar Wilbers <[email-masked]>
Date: Tue, 13 Jan 2009 10:11:50 +0100

Documentation is vital. I would be happy to contribute to that as well 
as look at the remaining issues, but that wouldn't be before March.

ilmar

---

From: Johan Hake <[email-masked]>
Date: Tue, 13 Jan 2009 10:14:19 +0100

> As always, improved documentation and better tests.

I have expanded the kwargs in build() with swig_include_dirs, wo be able 
to %import other swig modules. I needed this for the shared_ptr stuff. This 
will not break anything, so I can send a patch now or I can wait. You deside.

Johan

---

From: Johan Hake <[email-masked]>
Date: Tue, 13 Jan 2009 10:17:26 +0100

> that may be removed or must be explained.

Also, what is the difference between additional_declarations and 
additional_definitions? It seems that both gets included before the 
main %include of the the source, in the constructoed interface file. Whould 
it be instructive to have a pre_declaration and a a post_declaration as we 
have both in pycc and dolfin?

Johan

> build.py:        instant_error("FIXME: setup.py doesn't use the C
> sources.") codegeneration.py:      - additional_definitions (FIXME:
> comment)

---

From: "Martin Sandve Alnæs" <[email-masked]>
Date: Tue, 13 Jan 2009 10:33:28 +0100

Definitions are also included in the %{ %} section.
Definitions and declarations have specific meanings in C/C++,
but I'm not sure how clear that distinction is in their use here:

    interface_string = reindent("""
        %%module  %(modulename)s
        //%%module (directors="1") %(modulename)s

        //%%feature("director");

        %%{
        #include <iostream>
        %(additional_definitions)s
        %(system_headers_code)s
        %(local_headers_code)s
        %(wrap_headers_code1)s
        %(code)s
        %%}

        //%%feature("autodoc", "1");
        %%init%%{
        %(init_code)s
        %%}

        %(additional_definitions)s
        %(additional_declarations)s
        %(wrap_headers_code2)s
        //%(typemaps)s
        %(code)s;

        """ % locals())

Martin

---

From: "Martin Sandve Alnæs" <[email-masked]>
Date: Tue, 13 Jan 2009 10:39:48 +0100

Just thinking loudly here:

This is a declaration:

  void foo(int bar);

This is a definition:

  void foo(int bar)
  {
    cout << bar;
  }

SWIG only needs the declaration, but can take the definition as well
(just throws away the garbage).

Since additional_definitions is included together with additional_declarations
in the "non-%{}" section, additional_declarations is intended for
stuff that is only
declared for wrapping by SWIG while additional_definitions is intended for
stuff that needs to be compiled into the SWIG module _and_ wrapped by SWIG.

additional_declarations can depend on additional_definitions, but not
the other way around.

Martin

---

From: Kent Andre <[email-masked]>
Date: Tue, 13 Jan 2009 10:47:47 +0100

Correct, 
Definitions is defined within %{ %} and 
also wrapped later on. Declarations are only
wrapped. 

We need to state this clearly in the documentation. 

We may include pre and post directives as well if it
is needed. But I don't think we should support all of SWIG. 

Kent

---

From: Kent Andre <[email-masked]>
Date: Tue, 13 Jan 2009 10:51:46 +0100

> will not break anything, so I can send a patch now or I can wait. You deside.

I've used swigopt to set swig_include_dirs, but it may not be obvious. 

Kent

---

From: "Martin Sandve Alnæs" <[email-masked]>
Date: Tue, 13 Jan 2009 10:56:16 +0100

> I've used swigopt to set swig_include_dirs, but it may not be obvious.

I think that's fine, and we probably don't want to duplicate all
possible commandline options...

Martin

---

From: Johan Hake <[email-masked]>
Date: Tue, 13 Jan 2009 11:05:50 +0100

On Tuesday 13 January 2009 10:39:48 Martin Sandve Alnæs wrote:
> Since additional_definitions is included together with
> additional_declarations in the "non-%{}" section, additional_declarations
> is intended for
> stuff that is only

Mak sense.

Johan

> >> main %include of the the source, in the constructoed interface file.
> >> Whould it be instructive to have a pre_declaration and a a
> >> post_declaration as we have both in pycc and dolfin?
> >>
> >> Johan
> >>
> >>> TODO:

---

From: Johan Hake <[email-masked]>
Date: Tue, 13 Jan 2009 11:11:11 +0100

On Tuesday 13 January 2009 10:56:16 Martin Sandve Alnæs wrote:
> >> to %import other swig modules. I needed this for the shared_ptr stuff.
> >> This will not break anything, so I can send a patch now or I can wait.
> >> You deside.
> >>
> >> Johan
> >

The problem with swigargs (not swigopt) is that this comes with a lot 
predfined options, which all make sense, e.g., -c++, -I. (well, we could 
skip -fcompact and -small, makeing the produced wrapper code readable). 

When I then put e.g., [-I/path/to/something] as a swigargs it will exclude the 
rest. This is not a big issue, but it was not obvious that I should put 
include directories in swig_args. 

Having one argument for general swig options and one for include directories 
didn't seem too overkill for me.

Johan

---

From: "Martin Sandve Alnæs" <[email-masked]>
Date: Tue, 13 Jan 2009 11:16:22 +0100

> didn't seem too overkill for me.

Ok by me. I guess this is consistent with the C++ includes vs flags as well.

Martin

---

From: Kent Andre <[email-masked]>
Date: Tue, 13 Jan 2009 11:20:01 +0100

> Ok by me. I guess this is consistent with the C++ includes vs flags as well.

Fine by me to. 

Kent