Thanks. I was worrying about that as I scanned through the afw ticket (DM-6296).

Tim Jenness right, I think rebasing is beyond the scope of that ticket, since it involves going through all afw dependencies first.

base is done.
Note that all work is done on tickets/DM-6168.

pex_exceptions is done.

log is done.

utils is done. Had to bring over a missing test for Swig specific things (now testPybind11.py).
Note that some tests in that test file are currently disabled because pybind11 does not support them and it is unclear if we want or need it.

daf_base is done.

pex_policy is done.

pex_logging is done.

pex_config is done.

daf_persistence is done.

Rebase is complete on ticket branch for DM-6168. Python 2 and 3 builds work up to (and including part of) afw.
Because the work primarily involved rebasing DM-6168 the review procedure is a bit unclear. You can either run CI up to afw again to confirm or take a look at the few cleanup Python 3 commits on the various DM-6168 branches (I can provide a list if needed).
If you have another suggestion I'm also fine with that.
John Swinbank In general I suggest we will rebase all pybind11 work onto latest master on a weekly basis from now on. This will be a burden, but less then a full rebase in the end would be.

Can you create the relevant pull requests for review then? I'll do a local build on py3 but do I use DM-6168 or DM-7519?

So this review will be a little strange. Because I don't know how to make a PR for a rebase and Fred Moolekamp is already working on the branch for DM-6168 with the changes. So instead I have created an inverse PR using DM-7519 as a base with the cleanup commits undone. So the changes read as if they would be applied to DM-7519 but in fact they have already been applied to DM-6168. Any comments on the PR will be incorporated as new commits on DM-6168. I hope this is ok. For the future rebasing work we will need better procedures.

As for building. You should be able to simply do a rebuild -r tickets/DM-6168 to verify.

Ok. The PR is very opaque for something that's meant to be a minor change. I've tried rebuilding as you request and on my Mac it crashes in daf_persistence.

tests/DbStorage_1.py

E..

======================================================================

ERROR: testWriteRead (__main__.DbStorage1TestCase)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "tests/DbStorage_1.py", line 42, in setUp

    self.db = dafPers.DbStorage()

AttributeError: module 'lsst.daf.persistence' has no attribute 'DbStorage'

----------------------------------------------------------------------

Ran 3 tests in 0.076s

FAILED (errors=1)

I haven't got time to look at it further.

https://github.com/lsst/afw/pull/102/files is much cleaner. Thanks. Looks fine. Are you wanting reviews of all 7 branches?

The real problem with reviewing, is working out which changes are real changes by you and which changes are changes on master that are just finding themselves into the PR. It's a bit tricky to work that out. I'm going to trust that if everything builds on python 3 (let me know when to try again) and python 2 then things are fine (the problem is that you can't test all of python 2 and python 3 – just afw). There is a risk that we will drop a fix but maybe that will be more obvious when the final rebase/PR on master is ready.

I'm sorry, the PR was indeed a bit more messy than intended. I cleaned it up now.
I also just tested a rebuild -r tickets/DM-6168 afw which works without problems on my Mac (Sierra).
Can you please have second look? If you don't have time for the review, that is no problem I can ask someone else.
Again, sorry for the inconvenience.

I tried building again and I get the same failure. I also see:

python/lsst/daf/persistence/storage.cc:18:71: warning: 'base' is deprecated: base<T>() was deprecated in favor of specifying 'T' as a template

      argument to class_ [-Wdeprecated-declarations]

    py::class_<Storage, std::shared_ptr<Storage>> cls(mod, "Storage", py::base<lsst::daf::base::Citizen>());

                                                                      ^

/Users/timj/work/lsstsw3/stack/DarwinX86/pybind11/tickets.DM-6168-g92cdf896c1/include/pybind11/attr.h:38:5: note: 'base' has been explicitly marked

      deprecated here

    base() { }

    ^

python/lsst/daf/persistence/storage.cc:18:71: warning: 'base' is deprecated: base<T>() was deprecated in favor of specifying 'T' as a template

      argument to class_ [-Wdeprecated-declarations]

    py::class_<Storage, std::shared_ptr<Storage>> cls(mod, "Storage", py::base<lsst::daf::base::Citizen>());

                                                                      ^

/Users/timj/work/lsstsw3/stack/DarwinX86/pybind11/tickets.DM-6168-g92cdf896c1/include/pybind11/attr.h:38:5: note: 'base' has been explicitly marked

      deprecated here

    base() { }

    ^

2 warnings generated.

The warnings are expected. The API changed because multiple inheritance is now supported by upstream. I still have to change to the new convention of specifying bases.
I am however worried about the failure. As of yet I can't reproduce it on either Python 2 or 3.

Am I building the right versions?

# BUILD ID: b2223

                swig: 3.0.10 (already installed).

       mariadbclient: 10.1.11.lsst3 (already installed).

              python: 0.0.6 (already installed).

                fftw: 3.3.4.lsst2 (already installed).

            pybind11: tickets.DM-6168-g92cdf896c1 (already installed).

               boost: 1.60.lsst1+1 (already installed).

                 apr: 1.5.2 (already installed).

             minuit2: 5.34.14 (already installed).

               eigen: 3.2.5.lsst2 (already installed).

             doxygen: 1.8.5.lsst1 (already installed).

             cfitsio: 3360.lsst4 (already installed).

            apr_util: 1.5.4 (already installed).

               scons: 2.5.0.lsst2+1 (already installed).

                 gsl: 1.16.lsst3 (already installed).

       python_future: 0.15.2+4 (already installed).

             astropy: 0.0.1.lsst2+2 (already installed).

              pyyaml: 3.11.lsst1+4 (already installed).

        python_d2to1: 0.2.12.lsst2+1 (already installed).

             log4cxx: 0.10.0.lsst6+1 (already installed).

       python_psutil: 4.1.0+5 (already installed).

               numpy: 0.0.3+1 (already installed).

     stsci_distutils: 0.3.7.lsst1+2 (already installed).

             ndarray: tickets.DM-6168-ga68b9f3fa7 (already installed).

              wcslib: 5.13.lsst1 (already installed).

          sconsUtils: tickets.DM-6168-gc71a5af9cb (already installed).

          matplotlib: 0.0.3+1 (already installed).

              pyfits: 3.4.0+9 (already installed).

                base: tickets.DM-6168-g375d34a76e ok (13.6 sec).

             afwdata: 2.2016.10+5 (already installed).

      pex_exceptions: tickets.DM-6168-g909885c75c ok (14.1 sec).

                 log: tickets.DM-6168-g1c87a4e726+1 ok (10.9 sec).

               utils: tickets.DM-6168-gffea5650a0 ok (17.7 sec).

            daf_base: tickets.DM-6168-g200d77cff6 ok (32.0 sec).

          pex_policy: tickets.DM-6168-g4f43c611fe ok (50.2 sec).

     daf_persistence: tickets.DM-6168-g40cf864c45+1 ERROR (48 sec).

I haven't tried python 3 but in python 2 "rebuild -r tickets/DM-7519 -r tickets/DM-7518 -r tickets/DM-6168" worked for me the other day.

I'll note that wasn't the rebuild command that Pim Schellart [X] told me to use.

That is right. But the other branches are not needed anymore. Just DM-6168.

Tim Jenness The only difference in the versions I see is that I have daf_persistence: tickets.DM-6168-g40cf864c45+2 .....ok (69.4 sec).. I assume the +2 isn't actually a version difference. So far I have not been able to reproduce the issue.

I just ran rebuild -r tickets/DM-6168 afw on OS X El Capitan in python 3 and it installed correctly. There were a few minor version differences from Tim's:

pyyaml: 3.11.lsst1+3 ok (4.1 sec)

python_psutil: 4.1.0+3 ok (3.8 sec)

0.3.7.lsst1+2 ok (4.6 sec)

pyfits: 3.4.0+7 ......ok (27.9 sec)

log: tickets.DM-6168-g1c87a4e726 ok (8.3 sec)

daf_persistence: tickets.DM-6168-g40cf864c45 ok (31.1 sec)

but nothing major.

I'll note that this was a fresh install (meaning I cloned lsstsw into a new folder and ran a rebuild on a new stack), other than a symbolic link to afwdata.

I can't reproduce this problem.
I have tried building with rebuild -r tickets/DM-6168 afw on my Mac with Python 2 and 3.
Additionally I have tried new Jenkins Python 2 and 3 builds (of afw) and both Fred Moolekamp and Nate Lust have built it without problems.
Unsure how to proceed.

Is it possible that neither you nor Nate Lust nor Fred Moolekamp have the lsst-db credentials? Are the daf_persistence tests skipping for you? That would explain everything. Pim Schellart [X] I'll send you the special file – you really need it for your testing (Jenkins does not have it so the DB tests are skipped).

Yes, that was it. The tests were skipped both on my machine and on Jenkins.
I added the credentials and added the missing wrapper for DbStorage.

With the caveat that I haven't checked every line and I assume the code that exists on master is now back on the branch, this all looks fine and my build of afw did complete on Python 3.