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

Add nb::is_flag() annotation to Counter::Flags #1870

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

nicholasjng
Copy link
Contributor

This saves us the definition of __or__, because we can just use the one from enum.IntFlag.

cc @hawkinsp for a review, since you mentioned this should be possible after flag enum support is added to nanobind, which it is now, starting in v2.2.0.

I did have to use both nb::is_arithmetic() and nb::is_flag() due to combination issues, but it seems the C++ side is aware of this:

// A helper for user code to create unforeseen combinations of Flags, without
// having to do this cast manually each time, or providing this operator.
Counter::Flags inline operator|(const Counter::Flags& LHS,
const Counter::Flags& RHS) {
return static_cast<Counter::Flags>(static_cast<int>(LHS) |
static_cast<int>(RHS));
}

This saves us the definition of `__or__`, because we can just use the
one from `enum.IntFlag`.
@nicholasjng
Copy link
Contributor Author

For completeness, the traceback when just using nb::is_flag():

Traceback (most recent call last):
  File "/Users/nicholasjunge/Workspaces/c++/benchmark/bindings/python/google_benchmark/example.py", line 140, in <module>
    benchmark.main()
    ~~~~~~~~~~~~~~^^
  File "/Users/nicholasjunge/Workspaces/c++/benchmark/.venv/lib/python3.13/site-packages/google_benchmark/__init__.py", line 136, in main
    return app.run(_run_benchmarks, argv=argv, flags_parser=_flags_parser)
           ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/nicholasjunge/Workspaces/c++/benchmark/.venv/lib/python3.13/site-packages/absl/app.py", line 308, in run
    _run_main(main, args)
    ~~~~~~~~~^^^^^^^^^^^^
  File "/Users/nicholasjunge/Workspaces/c++/benchmark/.venv/lib/python3.13/site-packages/absl/app.py", line 254, in _run_main
    sys.exit(main(argv))
             ~~~~^^^^^^
  File "/Users/nicholasjunge/Workspaces/c++/benchmark/.venv/lib/python3.13/site-packages/google_benchmark/__init__.py", line 132, in _run_benchmarks
    return _benchmark.RunSpecifiedBenchmarks()
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/nicholasjunge/Workspaces/c++/benchmark/bindings/python/google_benchmark/example.py", line 91, in custom_counters
    num_foo, Counter.kIsRate | Counter.kInvert
             ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
  File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/enum.py", line 1590, in __or__
    return self.__class__(value | other_value)
           ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/enum.py", line 722, in __call__
    return cls.__new__(cls, value)
           ~~~~~~~~~~~^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/enum.py", line 1197, in __new__
    raise exc
  File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/enum.py", line 1174, in __new__
    result = cls._missing_(value)
  File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/enum.py", line 1469, in _missing_
    raise ValueError(
    ...<2 lines>...
                ))
ValueError: <flag 'Flags'> invalid value -2147483647
    given 0b1 0000000000000000000000000000001
  allowed 0b1 0000000000000000000000000001111

@dmah42 dmah42 merged commit d99cdd7 into google:main Oct 28, 2024
80 checks passed
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.

2 participants