Commit Graph

2525 Commits

Author SHA1 Message Date
Henry Schreiner
931f66440f
ci: cancel in-progress on repeated pushes (#3370) 2021-10-17 21:40:10 -04:00
Jerome Robert
56b49c2be2
ci: fix mingw checks by pinning (#3375)
* Workaround for https://github.com/msys2/setup-msys2/issues/167
* Adapted from 05036cb24d
2021-10-17 21:38:41 -04:00
Ralf W. Grosse-Kunstleve
7c580586f8
Correct options on Eigen::MappedSparseMatrix & adding MSVC C4127 suppression around Eigen includes. (#3352)
* Adding MSVC C4127 suppression around Eigen includes.

* For MSVC 2015 only: also adding the C4127 suppression to test_eigen.cpp

* Copying original change from PR #3343, with extra line breaks to not run past 99 columns (our desired but currently not enforced limit).
2021-10-11 13:13:01 -07:00
pre-commit-ci[bot]
02c05573d9
[pre-commit.ci] pre-commit autoupdate (#3353)
updates:
- [github.com/PyCQA/flake8: 3.9.2 → 4.0.1](https://github.com/PyCQA/flake8/compare/3.9.2...4.0.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2021-10-11 16:03:19 -04:00
Bruce Merry
8a7c266d26
Fix make_key_iterator/make_value_iterator for prvalue iterators (#3348)
* Add a test showing a flaw in make_key_iterator/make_value_iterator

If the iterator dereference operator returns a value rather than a
reference (and that pair also does not *contain* references),
make_key_iterator and make_value_iterator will return a reference to a
temporary, causing a segfault.

* Fix make_key_iterator/make_value_iterator for prvalue iterators

If an iterator returns a pair<T1, T2> rather than a reference to a pair
or a pair of references, make_key_iterator and make_value_iterator would
return a reference to a temporary, typically leading to a segfault. This
is because the value category of member access to a prvalue is an
xvalue, not a prvalue, so decltype produces an rvalue reference type.
Fix the type calculation to handle this case.

I also removed some decltype parentheses that weren't needed, either
because the expression isn't one of the special cases for decltype or
because decltype was only used for SFINAE. Hopefully that makes the code
a bit more readable.

Closes #3347

* Attempt a workaround for nvcc
2021-10-11 08:35:39 -07:00
NaDDu
750e38dcfd
Update eval.h (#3344)
typo correction

pybind11/exec.h → pybind11/eval.h
2021-10-09 11:38:34 -04:00
Aaron Gokaslan
f4c81e0877
maint: Add additional linter-related pre-commit hooks (#3337)
* Add additional pygrep pre-commit hooks

* Remove useless noqas with hook

* Fix all single rst backticks

* Simplify mypy pre-commit hook with upstream fixes

* Add back missing comment

* Add one last pygrep hook
2021-10-08 08:38:04 -04:00
dependabot[bot]
ed09664f06
chore(deps): bump ilammy/msvc-dev-cmd from 1.9.0 to 1.10.0 (#3338)
Bumps [ilammy/msvc-dev-cmd](https://github.com/ilammy/msvc-dev-cmd) from 1.9.0 to 1.10.0.
- [Release notes](https://github.com/ilammy/msvc-dev-cmd/releases)
- [Commits](https://github.com/ilammy/msvc-dev-cmd/compare/v1.9.0...v1.10.0)

---
updated-dependencies:
- dependency-name: ilammy/msvc-dev-cmd
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2021-10-08 08:27:52 -04:00
Bruce Merry
47ed124f37
Fix some formatting in the v2.8.0 changelog (#3339)
[skipci]
2021-10-08 08:27:38 -04:00
Henry Schreiner
ba9f919b85
chore: get back to work after 2.8.0 2021-10-04 17:37:27 -04:00
Henry Schreiner
e315e1fe2b
Merge branch 'master' into stable 2021-10-04 17:10:45 -04:00
pre-commit-ci[bot]
97976c16fb
[pre-commit.ci] pre-commit autoupdate (#3325)
updates:
- [github.com/asottile/pyupgrade: v2.28.0 → v2.29.0](https://github.com/asottile/pyupgrade/compare/v2.28.0...v2.29.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2021-10-04 16:38:31 -04:00
Henry Schreiner
3747dc2c4f
Revert "All -DDOWNLOAD_EIGEN=OFF (to work around gitlab eigen outage)." (#3326)
This reverts commit 9f146a5622.
2021-10-04 16:38:15 -04:00
Henry Schreiner
c9a319c607
chore: version 2.8.0 final 2021-10-04 15:33:58 -04:00
Henry Schreiner
20aae3e61a ci: disable Eigen due to Cert issue on CentOS 2021-10-04 15:20:10 -04:00
Henry Schreiner
591db0b930 docs: update CHANGELOG for 2.8 2021-10-04 15:20:10 -04:00
Ralf W. Grosse-Kunstleve
9f146a5622 All -DDOWNLOAD_EIGEN=OFF (to work around gitlab eigen outage). 2021-10-04 10:06:12 -07:00
Henry Schreiner
d7a7edc12b tests: support Eigen configuration 2021-10-04 10:06:12 -07:00
Aaron Gokaslan
ad96655605
fix: replace free() with std::free() (#3321)
* Disambiguate free() to use std::free()

* Add cstdlib include
2021-10-03 20:15:37 -04:00
Bruce Merry
b3573ac961
feat: add .keys and .values to bind_map (#3310)
* Add `.keys` and `.values` to bind_map

Both of these implement views (rather than just iterators), and `.items`
is also upgraded to a view. In practical terms, this allows a view to be
iterated multiple times and have its size taken, neither of which works
with an iterator.

The views implement `__len__`, `__iter__`, and the keys view implements
`__contains__`. Testing membership also works in item and value views
because Python falls back to iteration. This won't be optimal
for item values since it's linear rather than O(log n) or O(1), but I
didn't fancy trying to get all the corner cases to match Python
behaviour (tuple of wrong types, wrong length tuple, not a tuple etc).

Missing relative to Python dictionary views is `__reversed__` (only
added to Python in 3.8). Implementing that could break code that binds
custom map classes which don't provide `rbegin`/`rend` (at least without
doing clever things with SFINAE), so I've not tried.

The size increase on my system is 131072 bytes, which is rather large
(5%) but also suspiciously round (2^17) and makes me suspect some
quantisation effect.

* bind_map: support any object in __contains__

Add extra overload of `__contains__` (for both the map itself and
KeysView) which takes an arbitrary object and returns false.

* Take py::object by const reference in __contains__

To keep clang-tidy happy.

* Removing stray `py::` (detected via interactive testing in Google environment).

Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
2021-10-01 09:24:36 -04:00
xaedes
b4e1ab8caa
Docs: Demonstrate non-enum internal types in example (#3314)
* Docs: Demonstrate non-enum internal types in example

Previously example only demonstrated internal enumeration type. 
To show that it works for other internal types the same way the example was updated with an additional struct Pet::Attributes type.

* [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>
2021-09-30 14:45:06 -04:00
Henry Schreiner
a1830d5e6e
docs: mention title conventions in PR template (#3313) 2021-09-29 06:18:58 -04:00
Philipp Bucher
c9bbf8d2ee docs: fix minor typo (#3311) 2021-09-28 10:17:52 -04:00
Philipp Bucher
71fd524135
docs: fix minor typo (#3311) 2021-09-28 10:09:38 -04:00
pre-commit-ci[bot]
6be64304b7
[pre-commit.ci] pre-commit autoupdate (#3312)
updates:
- [github.com/asottile/pyupgrade: v2.26.0 → v2.28.0](https://github.com/asottile/pyupgrade/compare/v2.26.0...v2.28.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2021-09-28 10:09:08 -04:00
Henry Schreiner
6bce3bd72e
docs: update CHANGELOG (#3304) 2021-09-24 23:48:38 -04:00
Henry Schreiner
d58699c9ab
fix(cmake): reduce chance for variable collision (#3302) 2021-09-24 17:47:03 -04:00
Jeremy Maitin-Shepard
62c4909cce
Add custom_type_setup attribute (#3287)
* Fix `pybind11::object::operator=` to be safe if `*this` is accessible from Python

* Add `custom_type_setup` attribute

This allows for custom modifications to the PyHeapTypeObject prior to
calling `PyType_Ready`.  This may be used, for example, to define
`tp_traverse` and `tp_clear` functions.
2021-09-24 12:08:22 -07:00
Wenzel Jakob
409be8336f CMake: react to python version changes
The new FindPython-based variant of the CMake scripts caches information
about the chosen Python version that can become stale. For example,
suppose I configure a simple pybind11-based project as follows

```
cmake -S . -B build -GNinja -DPython_ROOT=<path to python 3.8>
```

which will generate `my_extension.cpython-38-x86_64-linux-gnu.so`.
A subsequent change to the python version like

```
cmake -S . -B build -GNinja -DPython_ROOT=<path to python 3.9>
```

does not update all necessary build system information. In particular,
the compiled file is still called
`my_extension.cpython-38-x86_64-linux-gnu.so`.

This commit fixes the problem by detecting changes in
`Python_EXECUTABLE` and re-running Python as needed.

Note that the previous way of detecting Python does not seem to be
affected, it always specifies the right suffix.
2021-09-24 17:42:06 +02:00
Henry Schreiner
6ad3f874a7
fix(build): avoid a possible warning about shadowed variables and changing behaviors (#3220) 2021-09-23 15:42:16 -04:00
Henry Schreiner
21282e645a
feat: reapply fixed version of #3271 (#3293)
* Add make_value_iterator (#3271)

* Add make_value_iterator

This is the counterpart to make_key_iterator, and will allow
implementing a `value` method in `bind_map` (although doing so is left
for a subsequent PR).

I made a few design changes to reduce copy-and-paste boilerplate.
Previously detail::iterator_state had a boolean template parameter to
indicate whether it was being used for make_iterator or
make_key_iterator. I replaced the boolean with a class that determines
how to dereference the iterator. This allows for a generic
implementation of `__next__`.

I also added the ValueType and Extra... parameters to the iterator_state
template args, because I think it was a bug that they were missing: if
make_iterator is called twice with different values of these, only the
first set has effect (because the state class is only registered once).
There is still a potential issue in that the *values* of the extra
arguments are latched on the first call, but since most policies are
empty classes this should be even less common.

* Add some remove_cv_t to appease clang-tidy

* Make iterator_access and friends take reference

For some reason I'd accidentally made it take a const value, which
caused some issues with third-party packages.

* Another attempt to remove remove_cv_t from iterators

Some of the return types were const (non-reference) types because of the
pecularities of decltype: `decltype((*it).first)` is the *declared* type
of the member of the pair, rather than the type of the expression. So if
the reference type of the iterator is `pair<const int, int> &`, then the
decltype is `const int`. Wrapping an extra set of parentheses to form
`decltype(((*it).first))` would instead give `const int &`.

This means that the existing make_key_iterator actually returns by value
from `__next__`, rather than by reference. Since for mapping types, keys
are always const, this probably hasn't been noticed, but it will affect
make_value_iterator if the Python code tries to mutate the returned
objects. I've changed things to use double parentheses so that
make_iterator, make_key_iterator and make_value_iterator should now all
return the reference type of the iterator. I'll still need to add a test
for that; for now I'm just checking whether I can keep Clang-Tidy happy.

* Add back some NOLINTNEXTLINE to appease Clang-Tidy

This is favoured over using remove_cv_t because in some cases a const
value return type is deliberate (particularly for Eigen).

* Add a unit test for iterator referencing

Ensure that make_iterator, make_key_iterator and make_value_iterator
return references to the container elements, rather than copies. The
test for make_key_iterator fails to compile on master, which gives me
confidence that this branch has fixed it.

* Make the iterator_access etc operator() const

I'm actually a little surprised it compiled at all given that the
operator() is called on a temporary, but I don't claim to fully
understand all the different value types in C++11.

* Attempt to work around compiler bugs

https://godbolt.org/ shows an example where ICC gets the wrong result
for a decltype used as the default for a template argument, and CI also
showed problems with PGI. This is a shot in the dark to see if it fixes
things.

* Make a test constructor explicit (Clang-Tidy)

* Fix unit test on GCC 4.8.5

It seems to require the arguments to the std::pair constructor to be
implicitly convertible to the types in the pair, rather than just
requiring is_constructible.

* Remove DOXYGEN_SHOULD_SKIP_THIS guards

Now that a complex decltype expression has been replaced by a simpler
nested type, I'm hoping Doxygen will be able to build it without issues.

* Add comment to explain iterator_state template params

* fix: regression in #3271

Co-authored-by: Bruce Merry <1963944+bmerry@users.noreply.github.com>
2021-09-23 15:06:07 -04:00
Jeremy Maitin-Shepard
2a78abffd8
Ensure PYBIND11_TLS_REPLACE_VALUE evaluates its arguments only once (#3290) 2021-09-23 13:36:25 -04:00
Henry Schreiner
5f46e47da8
tests: check simple iteration of pairs (#3296) 2021-09-23 08:01:06 -04:00
Henry Schreiner
2fa3fcfda5 Revert "Add make_value_iterator (#3271)"
This reverts commit ee0c5ee405.
2021-09-22 23:10:03 -04:00
dependabot[bot]
1dc9a23cae
chore(deps): bump jwlawson/actions-setup-cmake from 1.10 to 1.11 (#3294)
Bumps [jwlawson/actions-setup-cmake](https://github.com/jwlawson/actions-setup-cmake) from 1.10 to 1.11.
- [Release notes](https://github.com/jwlawson/actions-setup-cmake/releases)
- [Commits](https://github.com/jwlawson/actions-setup-cmake/compare/v1.10...v1.11)

---
updated-dependencies:
- dependency-name: jwlawson/actions-setup-cmake
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2021-09-22 22:38:04 -04:00
Henry Schreiner
b06a6f4f62
feat: Slice allowing None with py::object or std::optional (#1101)
* Adding nullptr slices

Using example from #1095

Some fixes from @wjakob's review

Stop clang-tidy from complaining

New proposal for py::slice constructor

Eric's suggested changes: simplify testing; shift def's

* chore: drop MSVC pragma (hopefully unneeded)

* Apply suggestions from code review
2021-09-22 17:41:56 -04:00
Aaron Gokaslan
0fb981b219
Add blacken-docs and pycln pre-commit hooks (#3292)
* Apply blacken-docs and fix language-hints

* Add blacken-docs pre-commit hook

* Add pycln pre-commit hook

* Enable a few builtin hooks

* Black no longer ignores pyi files
2021-09-22 15:38:50 -04:00
Bruce Merry
ee0c5ee405
Add make_value_iterator (#3271)
* Add make_value_iterator

This is the counterpart to make_key_iterator, and will allow
implementing a `value` method in `bind_map` (although doing so is left
for a subsequent PR).

I made a few design changes to reduce copy-and-paste boilerplate.
Previously detail::iterator_state had a boolean template parameter to
indicate whether it was being used for make_iterator or
make_key_iterator. I replaced the boolean with a class that determines
how to dereference the iterator. This allows for a generic
implementation of `__next__`.

I also added the ValueType and Extra... parameters to the iterator_state
template args, because I think it was a bug that they were missing: if
make_iterator is called twice with different values of these, only the
first set has effect (because the state class is only registered once).
There is still a potential issue in that the *values* of the extra
arguments are latched on the first call, but since most policies are
empty classes this should be even less common.

* Add some remove_cv_t to appease clang-tidy

* Make iterator_access and friends take reference

For some reason I'd accidentally made it take a const value, which
caused some issues with third-party packages.

* Another attempt to remove remove_cv_t from iterators

Some of the return types were const (non-reference) types because of the
pecularities of decltype: `decltype((*it).first)` is the *declared* type
of the member of the pair, rather than the type of the expression. So if
the reference type of the iterator is `pair<const int, int> &`, then the
decltype is `const int`. Wrapping an extra set of parentheses to form
`decltype(((*it).first))` would instead give `const int &`.

This means that the existing make_key_iterator actually returns by value
from `__next__`, rather than by reference. Since for mapping types, keys
are always const, this probably hasn't been noticed, but it will affect
make_value_iterator if the Python code tries to mutate the returned
objects. I've changed things to use double parentheses so that
make_iterator, make_key_iterator and make_value_iterator should now all
return the reference type of the iterator. I'll still need to add a test
for that; for now I'm just checking whether I can keep Clang-Tidy happy.

* Add back some NOLINTNEXTLINE to appease Clang-Tidy

This is favoured over using remove_cv_t because in some cases a const
value return type is deliberate (particularly for Eigen).

* Add a unit test for iterator referencing

Ensure that make_iterator, make_key_iterator and make_value_iterator
return references to the container elements, rather than copies. The
test for make_key_iterator fails to compile on master, which gives me
confidence that this branch has fixed it.

* Make the iterator_access etc operator() const

I'm actually a little surprised it compiled at all given that the
operator() is called on a temporary, but I don't claim to fully
understand all the different value types in C++11.

* Attempt to work around compiler bugs

https://godbolt.org/ shows an example where ICC gets the wrong result
for a decltype used as the default for a template argument, and CI also
showed problems with PGI. This is a shot in the dark to see if it fixes
things.

* Make a test constructor explicit (Clang-Tidy)

* Fix unit test on GCC 4.8.5

It seems to require the arguments to the std::pair constructor to be
implicitly convertible to the types in the pair, rather than just
requiring is_constructible.

* Remove DOXYGEN_SHOULD_SKIP_THIS guards

Now that a complex decltype expression has been replaced by a simpler
nested type, I'm hoping Doxygen will be able to build it without issues.

* Add comment to explain iterator_state template params
2021-09-21 13:37:19 -04:00
pre-commit-ci[bot]
077a16e9d1
[pre-commit.ci] pre-commit autoupdate (#3286)
updates:
- [github.com/psf/black: 21.8b0 → 21.9b0](https://github.com/psf/black/compare/21.8b0...21.9b0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2021-09-20 16:18:09 -04:00
Aaron Gokaslan
6e6975e217
Fix test case with __new__ (#3285) 2021-09-20 16:03:21 -04:00
Aaron Gokaslan
d0f3c51f01
Enable defining custom __new__ (#3265)
* Enable defining custom __new__

* See if xfail needed

* Qualify auto self

* Unconditionally defining PYBIND11_DISABLE_NEW_STYLE_INIT_WARNING. Returning pointer from "__init__" instead of reference.

* Use new style __init__

* Simplify __new__ creation

* Reviewer suggestions

* Match indentation

Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
2021-09-20 10:42:14 -04:00
Jeremy Maitin-Shepard
14976c853b
Eliminate duplicate TLS keys for loader_life_support stack (#3275)
* Eliminate duplicate TLS keys for loader_life_support stack

This revises the existing fix for
https://github.com/pybind/pybind11/issues/2765 in
https://github.com/pybind/pybind11/pull/3237 to reduce the amount of
TLS storage used.

The shared TLS key is stored in two different ways, depending on
`PYBIND11_INTERNALS_VERSION`.  If `PYBIND11_INTERNALS_VERSION ==
4` (as is currently set), the TLS key is stored in the
`internal::shared_data` map to avoid breaking ABI compatibility.  If
`PYBIND11_INTERNALS_VERSION > 4`, the TLS key is stored directly in
the `internals` struct.

* Fix test_pytypes.py::test_issue2361 failure on PyPy3.7

* Add github actions tests for unstable ABI
2021-09-20 04:57:38 -07:00
Henry Schreiner
04dd3262f0
docs: update CHANGELOG (#3276) 2021-09-17 17:28:26 -04:00
Matthias Köppe
e0031bfceb include/pybind11/numpy.h: gcc 4.8.4 does not have is_trivially_copyable (#3270) 2021-09-15 14:01:24 -07:00
pre-commit-ci[bot]
3ed31e9289
[pre-commit.ci] pre-commit autoupdate (#3266)
updates:
- [github.com/asottile/pyupgrade: v2.25.0 → v2.26.0](https://github.com/asottile/pyupgrade/compare/v2.25.0...v2.26.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2021-09-13 15:58:30 -04:00
Ralf W. Grosse-Kunstleve
6c65ab5950
Follow-on to PR #3254, to address user code breakages. (#3263)
* Restoring `const` removed from pytypes.h in PR #3254, adding tests reflective of user code that breaks when those `const` are removed.

* clang-tidy NOLINTs (and one collateral fix).

* Inserting PYBIND11_CONST_FOR_STRICT_PLATFORMS

* Trying `defined(__APPLE__)`

* Trying again: `auto it` for strict platforms.

* Adding NOLINTNEXTLINE(bugprone-macro-parentheses), expanding comments.

* Labeling all changes with `PR #3263`, for easy reference, and to make it easy to undo these changes if we decide to do so in the future.
2021-09-12 19:53:26 -07:00
Aaron Gokaslan
9978ed588b
Fix capsule bug (#3261)
Thanks Aaron for jumping in fixing this!
2021-09-10 11:23:32 -07:00
Laramie Leavitt
0e599589fe
Fix thread safety for pybind11 loader_life_support (#3237)
* Fix thread safety for pybind11 loader_life_support

Fixes issue: https://github.com/pybind/pybind11/issues/2765

This converts the vector of PyObjects to either a single void* or
a per-thread void* depending on the WITH_THREAD define.

The new field is used by each thread to construct a stack
of loader_life_support frames that can extend the life of python
objects.

The pointer is updated when the loader_life_support object is allocated
(which happens before a call) as well as on release.

Each loader_life_support maintains a set of PyObject references
that need to be lifetime extended; this is done by storing them
in a c++ std::unordered_set and clearing the references when the
method completes.

* Also update the internals version as the internal struct is no longer compatible

* Add test demonstrating threading works correctly.

It may be appropriate to run this under msan/tsan/etc.

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

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

* Update test to use lifetime-extended references rather than
std::string_view, as that's a C++ 17 feature.

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

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

* Make loader_life_support members private

* Update version to dev2

* Update test to use python threading rather than concurrent.futures

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

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

* Remove unnecessary env in test

* Remove unnecessary pytest in test

* Use native C++ thread_local in place of python per-thread data structures to retain compatability

* clang-format test_thread.cpp

* Add a note about debugging the py::cast() error

* thread_test.py now propagates exceptions on join() calls.

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

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

* remove unused sys / merge

* Update include order in test_thread.cpp

* Remove spurious whitespace

* Update comment / whitespace.

* Address review comments

* lint cleanup

* Fix test IntStruct constructor.

* Add explicit to constructor

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com>
2021-09-10 12:29:21 -04:00
Ralf W. Grosse-Kunstleve
121b91f99c
Fixing NOLINT mishap (#3260)
* Removing NOLINT pointed out by Aaron.

* Removing another NOLINT.
2021-09-10 10:16:09 -04:00
Aaron Gokaslan
ae07d4c6c6
maint(Clang-Tidy): readability-const-return (#3254)
* Enable clang-tidy readability-const-return

* PyTest functional

* Fix regression

* Fix actual regression

* Remove one more NOLINT

* Update comment
2021-09-09 21:27:36 -07:00