Add helper explicit_graph_inputs
#712
Conversation
|
Small note, in your top level message, the raw for This however misses the automatic closing of issues: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword It should just be Once you do that the |
ricardoV94
changed the title
required_graph_inputs
Dhruvanshu-Joshi
force-pushed
the
ticket3
branch
from
ceb891d to
85dd2d3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #712 +/- ##
=======================================
Coverage 80.78% 80.78%
=======================================
Files 162 162
Lines 46757 46762 +5
Branches 11440 11442 +2
=======================================
+ Hits 37773 37778 +5
+ Misses 6735 6732 -3
- Partials 2249 2252 +3
|
pytensor/graph/basic.py
Outdated
| Parameters | ||
| ---------- | ||
| graph: PyTensor `Variable` instances | ||
| Output `Variable` instances from which to search backward through | ||
| owners. | ||
|
|
||
| Returns | ||
| ------- | ||
| List of tensor variables that are input nodes with no owner, in the order | ||
| found by a left-recursive depth-first search started at the nodes in `graphs`. | ||
|
|
There was a problem hiding this comment.
These docstrings are not well formatted. @OriolAbril can we point to the same docs as for PyMC for PyTensor docs?
There was a problem hiding this comment.
not sure what you mean, link to numpydoc? https://numpydoc.readthedocs.io/en/latest/format.html
There was a problem hiding this comment.
I referred the format that pymc uses for logp. Does it seem right?
There was a problem hiding this comment.
not sure what you mean, link to numpydoc? https://numpydoc.readthedocs.io/en/latest/format.html
I was asking if we were using a style in PyTensor. I think you answered it
There was a problem hiding this comment.
oh, yes, same style as in pymc, numpydoc
Dhruvanshu-Joshi
force-pushed
the
ticket3
branch
2 times, most recently
from
8634a55 to
efa088a
Compare
Dhruvanshu-Joshi
force-pushed
the
ticket3
branch
from
efa088a to
73db222
Compare
Dhruvanshu-Joshi
force-pushed
the
ticket3
branch
from
73db222 to
15905f4
Compare
pytensor/graph/basic.py
Outdated
| @@ -936,6 +936,55 @@ def graph_inputs( | |||
| yield from (r for r in ancestors(graphs, blockers) if r.owner is None) | |||
|
|
|||
|
|
|||
| def explicit_graph_inputs( | |||
| graph: Variable[Any, Any] | Iterable[Variable[Any, Any]], | |||
There was a problem hiding this comment.
Do we need the Any, Any? The function above doesn't have it. Tbh I never had time to check where do these Any come from
| graph: Variable[Any, Any] | Iterable[Variable[Any, Any]], | |
| graph: Variable | Iterable[Variable], |
There was a problem hiding this comment.
This is what mypy suggested to add as type-hints when the first one that I was trying failed. However, I don't see reason for as to why Varaible in itself as a type-hint won't work. Would you suggest I revert to Variable instead of Variable[Any, Any]. Mypy seems to be happy with both.
ricardoV94
changed the title
required_graph_inputsexplicit_graph_inputs
Description
This PR aims to integrate a helper that builds on the current graph_input helper to return only variable inputs as in pymc.pytensorf.inputvars .
Related Issue
Checklist
Type of change