Skip to content
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

Merge 'upstream/master' (70a58c5) #57

Merged

Conversation

EricCousineau-TRI
Copy link
Collaborator

@EricCousineau-TRI EricCousineau-TRI commented Nov 24, 2021

For the author:

For the reviewer: There is no need to review upstream changes in detail. Instead, simply review:

  • Does the PR title look correct? (does it only contain up to
    <sha_new> of upstream?)
  • Are the canary builds "correct"? (look at the checks; for each CI configuration, expand the output
    under just Python tests (which are for C++17) - you should see around 500 tests run and passed)
    • CI / 🐍 3.6 • ubuntu-latest • x64 [...]: base vs. this PR's results for
      CI / 🐍 3.6 • ubuntu-latest • x64
    • CI / 🐍 3.9 • macos-latest • x64: base vs. this PR's results for
      CI / 🐍 3.9 • macos-latest • x64
  • Do the drake-specific changes look good? (all commits after the merge commit)

This change is Reviewable

Ralf W. Grosse-Kunstleve and others added 30 commits March 5, 2021 17:45
…ta to std::chrono::duration (pybind#2870)

* Use correct duration representation when casting from datetime.timdelta to std::chrono::duration

* When asserting datetime/timedelta/date/time we can equality-compare whole objects
* Adding suppression for pypocketfft.

* Generalize existing pypocketfft Valgrind suppression

Co-authored-by: Yannick Jadoul <[email protected]>
* Pybind11Extension add the "/EHsc /bigobj /std:c++14" flags on Windows.
  This is good for Visual C++ but not for Mingw.
* According
  https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-python2/0410-MINGW-build-extensions-with-GCC.patch
  sysconfig.get_platform() is the way to check for a Mingw64
…2923)

env: Add surrogate for pytest.deprecated_call for ptyest<3.9
* Adding PyGILState_Check() in object_api<>::operator().

* Enabling PyGILState_Check() for Python >= 3.6 only.

Possibly, this explains why PyGILState_Check() cannot safely be used with Python 3.4 and 3.5:

python/cpython#10267 (comment)

* Adding simple micro benchmark.

* Reducing test time to minimum (purely for coverage, not for accurate results).

* Fixing silly oversight.

* Minor code organization improvement in test.

* Adding example runtimes.

* Removing capsys (just run with `-k test_callback_num_times -s` and using `.format()`.
* chore: add myself to CODEOWNERS

This will ensure I get notified about pull requests where these files change.

* Update .github/CODEOWNERS
)

* Add a failure test for overloaded functions

* Allow function pointer extraction from overloaded functions
* correcting Werror for Intel

* adding ward for Intel

* adding wards for intel

* another ward for Intel

* missed one intel ward

* exact match for intel compiler

* removing inline limits

* disable warnings about inline limits

* formatter suggestion

* more indent

* hopefully make formatter happy

* addressed review

* fix &&

* Update tests/CMakeLists.txt

Co-authored-by: Henry Schreiner <[email protected]>

Co-authored-by: Henry Schreiner <[email protected]>
When the user defines _GLIBCXX_USE_CXX11_ABI=0 to force the pre-c++11 ABI, numpy.h assumes that is_trivially_copyable is available.
It is not necessarily the case. This patch uses clang's feature detection instead.
The workaround is for certain libstdc++ versions, so the test should target these particular versions.
Bumps [pre-commit/action](https://github.com/pre-commit/action) from v2.0.2 to v2.0.3.
- [Release notes](https://github.com/pre-commit/action/releases)
- [Commits](pre-commit/action@v2.0.2...9b88afc)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Crash when printing Unicode to redirected cout
Add failing tests

* Fix Unicode crashes redirected cout

* pythonbuf::utf8_remainder check end iterator

* Remove trailing whitespace and formatting iostream

* Avoid buffer overflow if ostream redirect races
This doesn't solve the actual race, but at least it now has a much lower
probability of reading past the end of the buffer even when data races
do occur.
test_call_policies: Explicitly check free-functions and static methods
* Set visibility of exceptions to default.

Co-authored-by: XZiar <[email protected]>

* add test

* update docs

* Skip failed test.
…failures that started to appear on 2020-05-27. (pybind#3022)
* Add const T to docstring generation.

* Change order.

* See if existing test triggers for a const type.

* Add tests.

* Fix test.

* Remove experiment.

* Reformat.

* More tests, checks run.

* Adding `test_fmt_desc_` prefix to new test functions.

* Using pytest.mark.parametrize to 1. condense test; 2. exercise all functions even if one fails; 3. be less platform-specific (e.g. C++ float is not necessarily float32).

Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
* Experiment using -DCMAKE_BUILD_TYPE=Debug for Centos 8.

* Moving comment because the current placement seems to mess up the cmake command.

* Using `echo > VAR_XXX` and `$(cat VAR_XXX)` trick to emulate using variables (actions/starter-workflows#68 (comment)).
Gjacquenot and others added 16 commits November 12, 2021 10:53
* Revert "style: drop pycln (pybind#3397)"

This reverts commit 606f81a.

* Update .pre-commit-config.yaml
* override: Fix wrong caching of the overrides

There was a problem when the python type, which was stored in override
cache for C++ functions, was destroyed and  the record wasn't removed from the
override cache. Therefor, dangling pointer was stored there. Then when the
memory was reused and new type was allocated at the given address and the
method with the same name (as previously stored in the cache) was actually
overridden in python, it would wrongly find it in the override cache for C++
functions and therefor override from python wouldn't be called.
The fix is to erase the type from the override cache when the type is destroyed.

* test: Pass by const ref instead of by value (clang-tidy)

* test: Rename classes and move to different files

Rename the classes and files so they're no too generic. Also, better place to
test the stuff is in test_virtual_functions.cpp/.py as we're basically testing
the virtual functions/trampolines.

* Add TODO for erasure code

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
updates:
- [github.com/PyCQA/isort: 5.10.0 → 5.10.1](PyCQA/isort@5.10.0...5.10.1)
- [github.com/shellcheck-py/shellcheck-py: v0.7.2.1 → v0.8.0.1](shellcheck-py/shellcheck-py@v0.7.2.1...v0.8.0.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* ci: support development releases of Python

* fix: better PyPy support

* fix: patch over a few more pypy issues

* Try to patch

* Properly follow pep667

* Fix typo

* Whoops, 667 not in yet

* For testing

* More testing

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Try to backport

* Try to simplify fix

* Nail down the fix

* Try pypy workaround

* Typo

* one last typo

* Replacing 0x03110000 with 0x030B0000

* Add TODO. Drop PyPy

* Fix typo

* Revert catch upgrade

* fix: minor cleanup, try pypy again

Co-authored-by: Aaron Gokaslan <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
* Fix py::kw_only when used before the first arg of a method

The implicit space for the `self` argument isn't added until we hit the
first argument, but this wasn't being done for kw_only or pos_only, and
so a kw_only before the first argument would break.

This fixes it by properly checking whether we need to add the self arg.

(The pos_only issue here was extremely mild -- you didn't get the `/` in
the docstring, but AFAICT it has no other effect since there are no
meaningful arguments before it anyway).

* Style changes

- rename check_have_self_arg -> append_self_arg_if_needed

- move the argument name inline comments before the args instead of
  after
* fix compiler warning: deprecated implicit copy constructor

* take care of the bug http://eigen.tuxfamily.org/bz/show_bug.cgi?id=747

* add parenthesis for better reading

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update include/pybind11/eigen.h

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <[email protected]>
* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/asottile/pyupgrade: v2.29.0 → v2.29.1](asottile/pyupgrade@v2.29.0...v2.29.1)
- [github.com/psf/black: 21.10b0 → 21.11b1](psf/black@21.10b0...21.11b1)
- [github.com/asottile/blacken-docs: v1.11.0 → v1.12.0](adamchainz/blacken-docs@v1.11.0...v1.12.0)

* Keep blacken-docs and black in sync.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Aaron Gokaslan <[email protected]>
)

* Replace usage of deprecated Eigen class

Eigen::MappedSparseMatrix has been deprecated since Eigen 3.3 from 2016. Use the equivalent modern syntax Eigen::Map<Eigen::SparseMatrix<...>>.

* Update eigen.h

* Update eigen.h
# Conflicts:
#	.github/workflows/ci.yml
#	.github/workflows/format.yml
#	.github/workflows/pip.yml
#	README.rst
#	docs/advanced/cast/overview.rst
#	include/pybind11/cast.h
#	include/pybind11/detail/type_caster_base.h
#	include/pybind11/eigen.h
#	include/pybind11/pybind11.h
#	tests/test_eigen.cpp
#	tests/test_multiple_inheritance.cpp
Copy link
Collaborator Author

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+@sherm1 for review, please!

The review here should be relatively lightweight; no need to review upstream changes, just do a sanity check (see checkboxes in overview).

Reviewable status: 0 of 177 files reviewed, all discussions resolved (waiting on @sherm1)

Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry Eric -- I don't understand the instructions:

  • "Does the commit title look correct?" There are 231 commits; to which one does this instruction refer?
  • Correctness of "canary builds": I don't know what that means. CI is all green.
  • Drake-specific changes (all after the merge commits): In the 2600-line Reviewable commit summary it appears that the last commit is a merge commit, with 11 files having merge conflicts: - eea11ec: Merge remote-tracking branch 'upstream/master' into feature-update. I don't know how to find the Drake-specific changes.

I would need something more like "instructions for dummies" to review this. Otherwise is there someone more knowledgeable who could review this with the current instructions?

Reviewed 5 of 177 files at r1.
Reviewable status: 5 of 177 files reviewed, all discussions resolved (waiting on @sherm1)

@EricCousineau-TRI
Copy link
Collaborator Author

Sorry about that.

  • I meant "PR title" - corrected that in the overview.
  • Basically, compare the output of the "Python tests" section between base (see link) and this PR. Does this screenshot help indicate what you should compare? https://github.com/RobotLocomotion/pybind11/blob/drake/README_DRAKE.md#pulling-upstream-changes (basically, just check that we didn't have a huge drop in number of tests run)
  • There aren't really any relevant Drake-specific changes; as mentioned above, all commits after merge commit, which there aren't any in this case. I've removed the bullet point in this case.

If this doesn't help illustrate / if you're still uncomfortable reviewing, I can also just self-stamp.

Thanks!

Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that helps -- thanks.

  • I verified that the PR title commit is the last one in November (doesn't include a few December ones).
  • I checked a few cases in CI and verified that lots of tests got run (~500).

So not much of a review, but FWIW :lgtm: !

Reviewable status: 5 of 177 files reviewed, all discussions resolved (waiting on @sherm1)

Copy link
Collaborator Author

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 177 of 177 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @EricCousineau-TRI)

@EricCousineau-TRI EricCousineau-TRI merged commit 5df3c00 into RobotLocomotion:drake Dec 8, 2021
@EricCousineau-TRI
Copy link
Collaborator Author

Awesome, thanks!

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.