Skip to content

refactor: remove unused generics_in_ptype/model, put TypedGraphRef.inputs_type before outputs_type#391

Merged
acl-cqc merged 7 commits intomainfrom
acl/tidy_types
Mar 5, 2026
Merged

refactor: remove unused generics_in_ptype/model, put TypedGraphRef.inputs_type before outputs_type#391
acl-cqc merged 7 commits intomainfrom
acl/tidy_types

Conversation

@acl-cqc
Copy link
Contributor

@acl-cqc acl-cqc commented Mar 4, 2026

Also remove some overloads that seem pointless (??), and tidy some imports

Removing generics_in_ptype as a preliminary to #381 because it's hard to handle parametrized FinishedGraph.

TypedGraphRef[Inputs, Outputs] was constructed via TypedGraphRef(outputs_type, inputs_type), so reorder the latter. (Although the inputs_type field is unused it helps pyright figure out the Inputs type parameter when in strict mode or with reportUnknownVariableType so keep it for now.)

@acl-cqc acl-cqc requested review from johnchildren and mwpb March 4, 2026 21:50
@acl-cqc acl-cqc changed the title refactor: remove unused generics_in_p(type,model) and TypedGraphRef.input_type refactor(types.rs): remove unused generics_in_p(type,model) and TypedGraphRef.input_type Mar 4, 2026
body: GraphBuilder[A, B] | TypedGraphRef,
eval_inputs: Any,
) -> Any:
) -> B:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly changing the type of parameter eval_inputs and the return type, both from Any, to A/B might have some impact???

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but I think it is a correct change though (and the lint passes) so I'm in favour of doing this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a very positive change

Copy link
Collaborator

@mwpb mwpb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good tidy up, thank you.

g = GraphBuilder(TKR[int], TKR[int])
graph = g.task(doubler_plus_graph())
graph_ref = TypedGraphRef(graph.value_ref(), TKR[int], TKR[int])
graph_ref = TypedGraphRef(graph.value_ref(), TKR[int])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pyright gives me a graph_ref is partially unknown warning here. However we have been overlooking these in other cases so I guess that is okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored inputs_type and put in correct order (before outputs) ;-)...we can try and solve in #381 by defining type TypedGraphRef[Inputs, Outputs] = TKR[FinishedGraph[Inputs, Outputs]]

body: GraphBuilder[A, B] | TypedGraphRef,
eval_inputs: Any,
) -> Any:
) -> B:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but I think it is a correct change though (and the lint passes) so I'm in favour of doing this.

@acl-cqc acl-cqc changed the title refactor(types.rs): remove unused generics_in_p(type,model) and TypedGraphRef.input_type refactor: remove unused generics_in_ptype/model, put TypedGraphRef.inputs_type before outputs_type Mar 5, 2026
@acl-cqc acl-cqc merged commit bad8066 into main Mar 5, 2026
13 checks passed
@acl-cqc acl-cqc deleted the acl/tidy_types branch March 5, 2026 17:49
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.

3 participants