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 Splitter and Tee classes #62

Open
wants to merge 1,824 commits into
base: main
Choose a base branch
from
Open

Conversation

rtburns-jpl
Copy link
Contributor

I like the idea of using a Tee for debugging - I can watch the messages in the terminal, but if I lose track and want to check something again I can go look in a file that stores those messages persistently.

But of course a Tee is just one instance of a more fundamental type of Device, a Splitter that appears to be a single Device interface-wise but actually forwards its contents, mapping inputs to any number of output Devices. In fact, the Tee specification is quite terse when phrased as a specific type of Splitter.

Let me know what you think, I'd like to know if I have the right understanding and am using appropriate abstractions here.

@rtburns-jpl rtburns-jpl force-pushed the splitter branch 2 times, most recently from 1ac68ce to 4fb6249 Compare August 27, 2020 07:23
@aivazis
Copy link
Member

aivazis commented Aug 27, 2020

on it. really cool, and thanks for the contribution!

@aivazis
Copy link
Member

aivazis commented Aug 29, 2020

I added stubs for the pure python implementations of Tee and Splitter. This was just a warmup exercise, trying to figure out what's a reasonable workflow for collaborating on a PR. Would it be better to merge it in a new pyre branch and continue the work on the pyre repository, or stay here and have me push contributions to your fork?

@rtburns-jpl
Copy link
Contributor Author

Cool, either works for me. Seems easier to just work on the PR branch as is, but let me know if any insufficiencies come up. I do like that we can discuss progress here as we work - it's like having a feature-specific discussion channel.

@aivazis
Copy link
Member

aivazis commented Apr 29, 2023

Merged pyre/main, removed the conflicts, updated the Splitter interface, and did some minor clean up in preparation to merge this PR. @rtburns-jpl, would you take a look and let me know whether i've missed anything?

@rtburns-jpl rtburns-jpl marked this pull request as ready for review May 1, 2023 20:35
@rtburns-jpl
Copy link
Contributor Author

Thanks, looks good

aivazis added 21 commits May 4, 2023 21:46
…s in a form suitable for including in diagnostics
…s in a form suitable for including in diagnostics
…ss in a form suitable for including in diagnostics
aivazis and others added 29 commits May 4, 2023 21:46
Co-authored-by: Sebastiaan van Paasen <sebas.98vp@gmail.com>
Co-authored-by: Sebastiaan van Paasen <sebas.98vp@gmail.com>
…ists.txt}

Specifically, this change required:
1) prepending "tests" to all the tests paths, and
2) for compiled tests, manually specifying the directory of the products of the compilation to the {tests} subdirectory of the build directory. This step is now necessary and was previously done automatically by CMake in function {add_subdirectory}, which however required the subdirectory to contain a CMakeLists.txt.
…clude} directive (extensions)

Specifically, this change required:
1) prepending "extensions" to all the paths, and
2) manually specifying the directory of the products of the compilation.

See also rev. 2dae115.
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.

None yet

3 participants