MNT: add EntryVisitor and refactor search methods to use SearchVisitor

[shilorigins]

WIP

• SearchVisitor is not a true generator, it just yields from a list to satisfy the existing generator handling
  • I'm working on seeing if I can make SearchVisitor a generator without requiring all EntryVisitors to be generators
  • Otherwise, I can adjust the .visit and .accept calls to support generators for visitors that want to use them

Description

Implement EntryVisitor abstract class, with corresponding Entry.accept methods
Implement SearchVisitor as the first concrete visitor
Add ancestor search option to SearchVisitor

Relates to #106

Motivation and Context

Using a visitor pattern will let us decouple Entry DAG traversal from specific components, such as the Client or concrete _Backend subclasses. This should make it easier to do things like collect and search entries, fill UUIDs, and save snapshots without having to re-implement traversal logic in multiple places.

How Has This Been Tested?

Existing search tests cover the new SearchVisitor.

Where Has This Been Documented?

Pre-merge checklist

• Code works interactively
• Code follows the style guide
• Code contains descriptive docstrings, including context and API
• New/changed functions and methods are covered in the test suite where possible
• Test suite passes locally
• Test suite passes on GitHub Actions
• Ran docs/pre-release-notes.sh and created a pre-release documentation page
• Pre-release docs include context, functional descriptions, and contributors as appropriate

[tangkong]

I think the presence and differentiation between .visit() and .visitEntryType() methods is throwing me off here, left a few rather confused comments. Perhaps consolidating everything with single-dispatch will make things simpler / smoother

[tangkong]

We can possibly use functools.singledispatch here to consolidate these methods. (as I familiarize myself more with this pattern I realize that's exactly what we should do)

[tangkong]

I'm actually confused by the presence of these two types of visit method, really all .visit() is doing here is calling Entry.accept(), which ultimately calls a .visit*() method? (In a sort of triple dispatch back and forth). Maybe all the .visit()'s are meant to be interchangeable?

[tangkong]

I'm seeing this elsewhere as starting with an "accept" call, such as root.acept(visitor). These are functionally equivalent right?

[tangkong]

Suggested change

	def _check_match(self, entry: Union[Entry, Root]) -> bool:
	def _check_match(self, entry: Union[Entry, Root]) -> None:

Because it confused me as I was reading this.

This would be the place to expose any entry that matches, and I think it might be worth bubbling that up and making visit yield as well. I can imagine other visitors gathering PVs or specific entries etc, but not every node we visit will yield a result (Entries that don't match). So in the end maybe only EntryVisitor.visit returns a generator and whatever other worker methods return values as necessary?

In untested skeleton code I'm imagining here (consolidating all the visit_ methods):

    def visit(self, entry: Union[Entry, Root, UUID]) -> None:
        if isinstance(entry, UUID):
            entry = self.backend.get_entry(entry)
        self.path.append(entry)

        # Perform work
        result = self._check_match(entry)
        if result is not None:
            yield result

        # visiting entry's children handled by Entry.accept() calls, previously in super().visit(entry)
        # but we get to visit via an `Entry.accept()` call, so why do we call accept again here?

        self.path.pop()

Excuse my general confusion here