This is a great start! However, in order to close #412, we need a bit more than that. Specifically,

1. The new functions currently cannot be called publicly (see the comment below).
2. It needs to verify that the arguments' format is correct. In this case, it amounts to checking if h is indeed a hypergraph (a list of lists). If not, it should produce an error message and return unevaluated.
3. We need a few tests (even though the functions are simple, we need to make sure they don't get broken in the future).
4. We need documentation for the new functions.

I also suggest putting these functions to a new file to distinguish from the other utilities which are for internal use within the package only.

So, we have two choices:

1. We can rename Subhypergraph -> subhypergraph and merge it so that the functions can be used internally only (for now).
2. If you'd like to do a bit more work, it would be best if you could address the comments above so that we can make the functions public and close Subhypergraph #412.

Thanks for contributing! If you have any questions, please don't hesitate to ask here or on our Discord server: https://discord.gg/QqjKHA9.

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @daneelsan)

Kernel/utilities.m, line 14 at r1 (raw file):

PackageScope["mapHold"]
PackageScope["heldPart"]
PackageScope["Subhypergraph"]

PackageScope means the symbols are only available within the package. Use PackageExport to make them public (but only if they are tested and documented).

Kernel/utilities.m, line 56 at r1 (raw file):

heldPart[expr_, part__Integer] := Extract[Unevaluated[expr], {part}, Hold]

Subhypergraph[h_, vertices_List] := Select[h, SubsetQ[vertices, #] &]

Private functions should have lowercase names, i.e., subhypergraph.

Kernel/utilities.m, line 58 at r1 (raw file):

Subhypergraph[h_, vertices_List] := Select[h, SubsetQ[vertices, #] &]

weakSubhypergraph[h_, vertices_List] := Select[h, ContainsAny[#, vertices] &]

If we are going to turn this into a public function, it will need to have an uppercase name, i.e., WeakSubhypergraph.

Excellent work! I just have a few comments, mostly about code style and invalid-argument messages, otherwise, it's ready to go!

Reviewed 4 of 4 files at r2.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @daneelsan)

README.md, line 2016 at r3 (raw file):

## Utility Functions

[WolframModelRuleValue](#wolframmodelrulevalue) | [GeneralizedGridGraph](#generalizedgridgraph) | [HypergraphAutomorphismGroup](#hypergraphautomorphismgroup) | [HypergraphUnifications](#hypergraphunifications) | [WolframPhysicsProjectStyleData](#wolframphysicsprojectstyledata) | [Build Data](#build-data) | [Subhypergraph](#subhypergraph)

I think we should move it to the beginning actually. I think it's going to be the most useful (or at least the least obscure) one.

README.md, line 2295 at r3 (raw file):

These constants are particularly useful for reporting issues with the code.

### Subhypergraph

If you move it in the index, also move the section itself above from here.

Kernel/Subhypergraph.m, line 1 at r3 (raw file):

(* ::Package:: *)

Please use the simplified format consistent with the other files (like https://github.com/maxitg/SetReplace/blob/master/Kernel/HypergraphUnificationsPlot.m). Specifically,

1. Don't use front-end generated comments like (* ::Package:: *), etc. They make the files unnecessarily long (see, e.g., how much shorted some files became after Fix WL code formatting #422).
2. Keep at most a single empty line at a time.
3. Use spaces instead of tabs.

Kernel/Subhypergraph.m, line 29 at r3 (raw file):

WeakSubhypergraph::usage = usageString[
	"WeakSubhypergraph[`h`, `vertices`] that selects any hyperedge from `h` whose elements are contained in `vertices`.",

I think the word "that" should not be here.

Kernel/Subhypergraph.m, line 54 at r3 (raw file):

hypergraphQ = MatchQ[#, {___List}]&;

Nit: space before &.

Kernel/Subhypergraph.m, line 115 at r3 (raw file):

expr: Subhypergraph[h_ ? (Not @* hypergraphQ), _] := 0 /;

We also need messages for the operator forms of both functions.

Kernel/Subhypergraph.m, line 116 at r3 (raw file):

expr: Subhypergraph[h_ ? (Not @* hypergraphQ), _] := 0 /;
	Message[Subhypergraph::invalidHypergraph, 1, HoldForm@expr]

Nit: Change HoldForm@expr -> HoldForm @ expr consistent with the rest of the codebase.

Kernel/Subhypergraph.m, line 120 at r3 (raw file):

expr: WeakSubhypergraph[h_ ? (Not @* hypergraphQ), _] := 0 /;
	Message[WeakSubhypergraph::invalidHypergraph, 1, HoldForm@expr]

Nit: same here.

Kernel/Subhypergraph.m, line 127 at r3 (raw file):

Subhypergraph[h_ , v : Except[_List]] := 0 /; False

Shouldn't this print a message as well?

Kernel/Subhypergraph.m, line 130 at r3 (raw file):

WeakSubhypergraph[h_ , v : Except[_List]] := 0 /; False

Same here.

Tests/Subhypergraph.wlt, line 11 at r3 (raw file):

      VerificationTest[
        Subhypergraph[{},{}],

Add spaces after commas, the same everywhere else in this file.

Tests/Subhypergraph.wlt, line 42 at r3 (raw file):

      testUnevaluated[
        Subhypergraph[1,{2,3,4}],
        {Subhypergraph::invalidHypergraph}

Check other kinds of invalid hypergraphs as well, namely, e.g., {1, 2}, i.e., depth 1 list instead of depth 2.

Tests/Subhypergraph.wlt, line 44 at r3 (raw file):

        {Subhypergraph::invalidHypergraph}
      ],

Also, test the not-yet implemented messages for the vertices, and the operator forms (both for the invalid hypergraph and the invalid vertex list).

Tests/Subhypergraph.wlt, line 88 at r3 (raw file):

      testUnevaluated[
        WeakSubhypergraph[1,{2,3,4}],

Same here, test depth-1 list as the first argument.

Good point. I updated the contributing guidelines just now (#440), including the links to the code with outdated formatting. Thanks for pointing that out!

Reviewed 3 of 3 files at r4.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @daneelsan)

Kernel/Subhypergraph.m, line 1 at r4 (raw file):

(* ::Package:: *)

Remove this (* ::Package:: *) line as well.

Kernel/Subhypergraph.m, line 37 at r4 (raw file):

(* main *)
expr : Subhypergraph[arg1_, arg2_] := With[{res = Catch[iSubhypergraph[HoldForm @ expr, arg1, arg2]]},

What does i mean in iSubhypergraph? Either expand to the full name or use the lowercase subhypergraph instead (which is what is done in the rest of the codebase).

Kernel/Subhypergraph.m, line 38 at r4 (raw file):

(* main *)
expr : Subhypergraph[arg1_, arg2_] := With[{res = Catch[iSubhypergraph[HoldForm @ expr, arg1, arg2]]},
	res /; res =!= $Failed

Change tabs to spaces.

Kernel/Subhypergraph.m, line 41 at r4 (raw file):

]

expr : WeakSubhypergraph[arg1_, arg2_] := With[{res = Catch[iWeakSubhypergraph[HoldForm @ expr, arg1, arg2]]},

Same about i here.

Kernel/Subhypergraph.m, line 42 at r4 (raw file):

expr : WeakSubhypergraph[arg1_, arg2_] := With[{res = Catch[iWeakSubhypergraph[HoldForm @ expr, arg1, arg2]]},
	res /; res =!= $Failed

Tabs -> spaces.

Kernel/Subhypergraph.m, line 47 at r4 (raw file):

(* operator form *)
expr : Subhypergraph[args0___][args1___] := With[{res = Catch[iSubhypergraph[HoldForm @ expr][args0][args1]]},
	res /; res =!= $Failed

Tabs -> spaces.

Kernel/Subhypergraph.m, line 51 at r4 (raw file):

expr : WeakSubhypergraph[args0___][args1___] := With[{res = Catch[iWeakSubhypergraph[HoldForm @ expr][args0][args1]]},
	res /; res =!= $Failed

Tabs -> spaces

Kernel/Subhypergraph.m, line 78 at r4 (raw file):

(** vertices **)
General::invalidVertexList =
  "The argument at position `1` in `2` is not a valid hypergraph.";

This message is never used.

Kernel/Subhypergraph.m, line 81 at r4 (raw file):

iSubhypergraph[_, _ , v : Except[_List]] :=
	(Message[Subhypergraph::invl, 2];

Tabs -> spaces

Kernel/Subhypergraph.m, line 89 at r4 (raw file):

(* operator form *)
iSubhypergraph[_][vertices_List][h_ ? hypergraphQ] := iSubhypergraph[None, h, vertices]

Why not just pass it as is (without checking) to the normal form? It's going to argument check everything except for the arguments count anyway.

Kernel/Subhypergraph.m, line 93 at r4 (raw file):

iWeakSubhypergraph[_][vertices_List][h_ ? hypergraphQ] := iWeakSubhypergraph[None, h, vertices]

(* The operator form of Select does not check that the zeroth argument is valid, it just returns {} *)

Is it the same thing here though? Select returns an empty list even in a normal form:

In[] := Select[{1, 5, 3}, 5]
Out[] = {}

I'm not sure why it does that, but it does not seem to have anything to do with its form. I think we should print the message because someone might accidentally use WeakSubhypergraph[3] assuming it will select all hyperedges containing vertex 3, and it would instead fail silently, which is dangerous.

Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @daneelsan and @maxitg)

Kernel/Subhypergraph.m, line 1 at r4 (raw file):

I didn't notice it because I was using Mathematica :-)

Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @daneelsan and @maxitg)

Kernel/Subhypergraph.m, line 1 at r4 (raw file):

Done.

Kernel/Subhypergraph.m, line 89 at r4 (raw file):

iWeakSubhyperg

I guess because if the check failed during the call to weaksubhypergraph, then the error message would show the incorrect "caller".

Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @daneelsan and @maxitg)

Kernel/Subhypergraph.m, line 78 at r4 (raw file):

Found the mistake!

Just one little thing left, and we are good to go!

Reviewed 2 of 2 files at r5.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @daneelsan)

Kernel/Subhypergraph.m, line 41 at r4 (raw file):

The convention is lower camel, not everything lowercase. So, it would be weakSubhypergraph.

Kernel/Subhypergraph.m, line 81 at r4 (raw file):

iSubhypergraph[_, _ , v : Except[_List]] :=
	(Message[Subhypergraph::invl, 2];

I don't see a problem using the build-in message (General::invl), but this works as well, I don't really have a preference. I think your message is better than the built-in one.

Kernel/Subhypergraph.m, line 89 at r4 (raw file):

I see, the position would be incorrect, makes sense.

Awesome work! Thanks for contributing! 🚀

Reviewed 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved