Note this repeats the pick_parent in try_new_with_checker (inside nodes_if_valid_and_convex), which is a little inefficient. We could just pick an arbitrary parent (and then either that would be ok, or we'd reject the boundary as invalid later anyway). I did this in b24107e but it seemed more complicated not less so I reverted...

Couldn't we split pick_parent into a picker and a checker?

Keep the old pick_parent, and rename your new version to check_parents (passing the parent as parameter instead), and only call it from nodes_if_valid_and_convex.

Ok, well, sort of, and sort of gone further.
Old pick_parent code reinstated, but inlined into its only use (in try_new, to create the checker).
pick_parent renamed to check_parent does all the checking...could rename, I mean it's private.

Moreover, moved responsibility for check_parent and checking against the checker's region_parent into try_new_with_checker, as otherwise every impl of nodes_if_valid_and_convex would have to do that. Hence, renamed to just nodes_if_convex (there is some validity checking still though....so could keep the longer name if you prefer). EDIT: I'm not so convinced about this anymore; we therefore also need to check the checker is appropriate in fn validate (when a checker is passed in), so two places. But, I dunno, it does feel like the better trait interface; thoughts?

Finally, I've dropped the check in validate_subgraph (now validate_subgraph_boundary) that all the nodes have the same parent. Whilst that sounds like a good thing, on closer inspection

1. It's not used in fn validate - at least not directly, not on the nodes in the SiblingSubgraph: they are checked only for equality against (recomputing what the nodes should be by the ConvexChecker or using make_pg_subgraph i.e. a portgraph flat region (subgraph))
2. It's used in try_new_dataflow_subgraph but here it's pretty trivial (all the nodes were taken from the same hugr region)
3. It's used in nodes_if_convex i.e. checking that the nodes just calculated by the ConvexityChecker (again using make_pg_subgraph) have the same parent. Which again seems a bit pointless.

So could revert that one (02ee279) if you feel strongly, but if you are on the fence - we are doing a stronger check, see point 1.

Hehe, fair enough.

I think the change is fine.

Couldn't we split pick_parent into a picker and a checker?

Keep the old pick_parent, and rename your new version to check_parents (passing the parent as parameter instead), and only call it from nodes_if_valid_and_convex.