tcbrindle / flux

A C++20 library for sequence-orientated programming
https://tristanbrindle.com/flux/
Boost Software License 1.0
476 stars 29 forks source link

Add mask adaptor #85

Closed tcbrindle closed 1 year ago

tcbrindle commented 1 year ago

Given a sequence of values and a sequence of selectors, select_by(vals, selectors) yields those elements of vals for which the corresponding element of selectors is true (after conversion to bool).

Python itertools calls this "compress", but I'm not overly keen on that name. On the other hand, select_by isn't ideal either -- chunk_by takes a predicate, but this takes a second sequence, so there's an inconsistency there. I did consider just calling it select(), but then selection algorithms are a thing...

Fixes #84

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (4d6aa03) 97.67% compared to head (583f208) 97.67%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #85 +/- ## ======================================= Coverage 97.67% 97.67% ======================================= Files 66 66 Lines 2237 2237 ======================================= Hits 2185 2185 Misses 52 52 ``` | [Impacted Files](https://app.codecov.io/gh/tcbrindle/flux/pull/85?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tristan+Brindle) | Coverage Δ | | |---|---|---| | [include/flux/core/inline\_sequence\_base.hpp](https://app.codecov.io/gh/tcbrindle/flux/pull/85?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tristan+Brindle#diff-aW5jbHVkZS9mbHV4L2NvcmUvaW5saW5lX3NlcXVlbmNlX2Jhc2UuaHBw) | `91.66% <ø> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

tcbrindle commented 1 year ago

Hmm, it looks like an update to MSVC or CMake in the Github Actions Windows image has broken stuff...

It appears that it's now scanning for module dependencies, which would be great except that we're not using modules and yet it appears to be recompiling the std.ixx and std.compat.ixx modules for every single target we have, which is slowing things down a lot.

I could live with that, except that it also seems to break our usage of precompiled headers (which we do to speed up #include-ing Catch in each test TU), even though we're using the built-in CMake target_precompiled_headers command to do it.

I'm not sure what's changed that's made it start doing this, but perhaps the easiest workaround for now is to disable precompiled headers on Windows builds and just put up with the slowdown.

jnytra commented 1 year ago

The code looks nice. As for the name of the adapter, I just prefer select(), but here are some other names that came to mind:

tcbrindle commented 1 year ago

Thanks for the name suggestions. I agree select is the nicest, but I quite like mask as well, and that doesn't have an existing meaning

tcbrindle commented 1 year ago

I had a chat with a few folks at C++ on Sea about the name, and I was reminded that Posix select() is a thing, which might be confusing. Sean Parent suggested the name mask(), so that seems like a good one to go for.

tcbrindle commented 1 year ago

The good news is that the Windows CI problems seem to have resolved themselves. The bad news is that now the MacOS CI isn't working because of problems with Homebrew sigh. Hopefully these will get sorted out too and we can merge this.

jnytra commented 1 year ago

I also like the mask() version the most.