Skip to content

Make a 2.2.2 release #1250

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

Merged
merged 26 commits into from
Feb 7, 2018
Merged

Make a 2.2.2 release #1250

merged 26 commits into from
Feb 7, 2018

Conversation

jagerman
Copy link
Member

I think we've accumulated more than enough fixes to make a 2.2.2 release; this PR (against the v2.2 branch) is just the appropriate commits cherry-picked from master (and doesn't add anything new aside from the changelog and version variables bump).

If anyone knows of some critical fix that I missed in this PR, or some other issue that needs to be fixed for 2.2.2, now is the time to speak up to get it included.

jagerman and others added 21 commits January 12, 2018 10:40
This changes the caster to return a reference to a (new) local `CharT`
type caster member so that binding lvalue-reference char arguments
works (currently it results in a compilation failure).

Fixes pybind#1116
This fixes a bug introduced in b68959e
when passing in a two-dimensional, but conformable, array as the value
for a compile-time Eigen vector (such as VectorXd or RowVectorXd).  The
commit switched to using numpy to copy into the eigen data, but this
broke the described case because numpy refuses to broadcast a (N,1)
into a (N).

This commit fixes it by squeezing the input array whenever the output
array is 1-dimensional, which will let the problematic case through.
(This shouldn't squeeze inappropriately as dimension compatibility is
already checked for conformability before getting to the copy code).
Building with the (VS2017) /permissive- flag puts the compiler into
stricter standards-compliant mode.  It shouldn't cause the compiler to
work differently--it just disallows some non-conforming code--so should
be perfectly fine for the test suite under all VS2017 builds.

This commit also fixes one failure under non-permissive mode.
- For the debian/buster docker build (GCC 7/C++17) install and use the
  system `catch` package; this also renames "COMPILER_PACKAGES" to
  "EXTRA_PACKAGES" since it now contains a non-compiler package.

- Add a status message indicating the catch version being used for
  compiling the embedded tests

- Simplify some bash code by using VAR+=" foo" to append (rather than
  VAR="${VAR} foo"

- Fix CMAKE_INCLUDE_PATH appending: it was prepending the ':' but not
  the existing $CMAKE_INCLUDE_PATH value and so would end up with
  ":/eigen-path" if CMAKE_INCLUDE_PATH was already set.  (This wasn't
  bug that was actually noticed since currently nothing else sets it).
The just-updated flake8 package hits a bunch of:

    E741 ambiguous variable name 'l'

warnings.  This commit renames them all from `l` to `lst` (they are all
list values) to avoid the error.
Non-user facing. 
Found using `codespell -q 3`
A few fixes related to how we set `__qualname__` and how we show the
type name in function signatures:

- `__qualname__` isn't supposed to have the module name at the
beginning, but we've been putting it there.  This removes it, while
keeping the `Nested.Class` name chaining.

- print `__module__.__qualname__` rather than `type->tp_name`; the
latter doesn't work properly for nested classes, so we would get
`module.B` rather than `module.A.B` for a class `B` with parent `A`.
This also unifies the Python 3 and PyPy code.  Fixes pybind#1166.

- This now sets a `__qualname__` attribute on the type (as would happen
in Python 3.3+) for Python <3.3, including PyPy.  While not particularly
important to have in earlier Python versions, it's useful for us to be
able to extracted the nested name, which is why `__qualname__` was
invented in the first place.

- Added tests for the above.
In the latest MSVC in C++17 mode including Eigen causes warnings:

    warning C4996: 'std::unary_negate<_Fn>': warning STL4008: std::not1(),
    std::not2(), std::unary_negate, and std::binary_negate are deprecated in
    C++17. They are superseded by std::not_fn(). You can define
    _SILENCE_CXX17_NEGATORS_DEPRECATION_WARNING or
    _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have
    received this warning.

This disables 4996 for the Eigen includes.

Catch generates a similar warning for std::uncaught_exception, so
disable the warning there, too.

In both cases this is temporary; we can (and should) remove the warnings
disabling once new upstream versions of Eigen and Catch are available
that address the warning. (The Catch one, in particular, looks to be
fixed in upstream master, so will probably be fixed in the next (2.0.2)
release).
The `py::args` or `py::kwargs` arguments aren't properly referenced
when added to the function_call arguments list: their reference counts
drop to zero if the first (non-converting) function call fails, which
means they might be cleaned up before the second pass call runs.

This commit adds a couple of extra `object`s to the `function_call`
where we can stash a reference to them when needed to tie their
lifetime to the function_call object's lifetime.

(Credit to YannickJadoul for catching and proposing a fix in pybind#1223).
When using the mixed position + vararg path, pybind over inc_ref's
the vararg positions. Printing the ref_count() of `item` before
and after this change you see:

Before change:

```
refcount of item before assign 3
refcount of item after assign 5
```

After change
```
refcount of item before assign 3
refcount of item after assign 4
```
- UPDATEIFCOPY is deprecated, replaced with similar (but not identical)
  WRITEBACKIFCOPY; trying to access the flag causes a deprecation
  warning under numpy 1.14, so just check the new flag there.
- Numpy `repr` formatting of floats changed in 1.14.0 to `[1., 2., 3.]`
  instead of the pre-1.14 `[ 1.,  2.,  3.]`.  Updated the tests to
  check for equality with the `repr(...)` value rather than the
  hard-coded (and now version-dependent) string representation.
The anonymous struct nested in a union triggers a -Wnested-anon-type
warning ("anonymous types declared in an anonymous union are an
extension") under clang (pybind#1204).  This names the struct and defines it
out of the definition of `instance` to get around to warning (and makes
the code slightly simpler).
Found via `codespell`
Fix return from `std::map` bindings to `__delitem__`: we should be returning `void`, not an iterator.

Also adds a test for map item deletion.
…#1092)

* Fix segfault when reloading interpreter with external modules

When embedding the interpreter and loading external modules in that
embedded interpreter, the external module correctly shares its
internals_ptr with the one in the embedded interpreter.  When the
interpreter is shut down, however, only the `internals_ptr` local to
the embedded code is actually reset to nullptr: the external module
remains set.

The result is that loading an external pybind11 module, letting the
interpreter go through a finalize/initialize, then attempting to use
something in the external module fails because this external module is
still trying to use the old (destroyed) internals.  This causes
undefined behaviour (typically a segfault).

This commit fixes it by adding a level of indirection in the internals
path, converting the local internals variable to `internals **` instead
of `internals *`.  With this change, we can detect a stale internals
pointer and reload the internals pointer (either from a capsule or by
creating a new internals instance).

(No issue number: this was reported on gitter by @henryiii and @aoloe).
@jagerman
Copy link
Member Author

Pushed a couple fixes for a new MSVC /permissive- error reported in #1239. (That this warning comes up in the first place smells like an MSVC bug, but the workaround is easy and allows removing a bunch of now-unnecessary detail:: qualifications).

This fixes pybind#1251 (patient vector grows without bounds) for the 2.2.2
branch by checking that the vector doesn't already have the given
patient.

This is a little less elegant than the same fix for `master` (which
changes the patients `vector` to an `unordered_set`), but that requires
an internals layout change, which this approach avoids.
This updates the `py::init` constructors to only use brace
initialization for aggregate initiailization if there is no constructor
with the given arguments.

This, in particular, fixes the regression in pybind#1247 where the presence of
a `std::initializer_list<T>` constructor started being invoked for
constructor invocations in 2.2 even when there was a specific
constructor of the desired type.

The added test case demonstrates: without this change, it fails to
compile because the `.def(py::init<std::vector<int>>())` constructor
tries to invoke the `T(std::initializer_list<std::vector<int>>)`
constructor rather than the `T(std::vector<int>)` constructor.

By only using `new T{...}`-style construction when a `T(...)`
constructor doesn't exist, we should bypass this by while still allowing
`py::init<...>` to be used for aggregate type initialization (since such
types, by definition, don't have a user-declared constructor).
@jagerman
Copy link
Member Author

Added 89cb495 (and updated the changelog), which is an equivalent to #1253 (for issue #1251), backported to 2.2 so as to leave the patients list as a vector (and walking through it to look for and avoid storing duplicate patients).

jagerman and others added 3 commits February 6, 2018 10:32
Current MSVC generates totally bizarre errors:

    error C2884: 'pybind11::detail::_': introduced by using-declaration
    conflicts with local function 'pybind11::detail::_'

which makes no sense (since the supposed "conflict" is the function
itself).  Work around it by `using namespace detail;` instead (which
also lets us drop a bunch of other `detail::` qualifications, so isn't
actually a bad thing).
@wjakob
Copy link
Member

wjakob commented Feb 7, 2018

This all looks good to me, I will push out the release shortly.

@wjakob wjakob merged commit 19e90dc into pybind:v2.2 Feb 7, 2018
@wjakob
Copy link
Member

wjakob commented Feb 7, 2018

Done. 2d0507d adds the changelog to the main branch.

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

Successfully merging this pull request may close these issues.

7 participants