Dependency graph #659
Dependency graph #659
Conversation
Signed-off-by: Polina Binder <[email protected]>
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #659 +/- ##
==========================================
- Coverage 86.65% 86.27% -0.38%
==========================================
Files 117 118 +1
Lines 7021 7162 +141
==========================================
+ Hits 6084 6179 +95
- Misses 937 983 +46 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I am not sure is pyproject.toml is the up to date source of dependency information anymore. I am not sure how it is maintained.
If no, I thin we should implement a script that parses dependency graph for subpackages from the scripts, ie parse the project.toml files of the main project and its subpackages, extract the import paths used in Python scripts under the src directory of each subpackage, and verifies all imports starting with from bionemo.
|
To me this looks like a good start just need to document "how to run" script in github PR description. I don't necessary need this to do any more than in currently does ( @pstjohn @dorotat-nv , I suggest we constrain our review just to "graph drawing" rather than any sort of py file parsing + CI enforcement. |
polinabinder1
force-pushed
the
polinabinder/package_dependencies
branch
2 times, most recently
from
ccab862 to
94fd399
Compare
Signed-off-by: Polina Binder <[email protected]>
polinabinder1
force-pushed
the
polinabinder/package_dependencies
branch
from
94fd399 to
ad206c4
Compare
Signed-off-by: Polina Binder <[email protected]>
Done! |
polinabinder1
requested review from
jstjohn,
malcolmgreaves,
sichu2023,
skothenhill-nv,
jomitchellnv,
jwilber and
cspades
as code owners
There was a problem hiding this comment.
This looks great, the module was laid out nicely and you have a lot of clear, reusable functions.
I do think we need unit tests for these functions though. Copilot could likely do that pretty quickly. Ideally any public function / class / method should get a test for all its expected behavior and edge cases, but even just a basic test for each of these functions would be great
I also think we should add those resulting images to our documentation; along with a command of how to run this to regenerate them.
Unfortunately putting this in internal/scripts means it's harder to write tests for; we don't execute pytests in that subdirectory. Maybe this could live in bionemo-fw? Or we could add that directory to our pytest call. But I'm worried that if we don't exercise this script in CI, it will break quickly without us realizing it and we won't be able to use it in planning a version bump / release strategy.
Signed-off-by: Polina Binder <[email protected]>
polinabinder1
requested review from
DejunL,
farhadrgh and
guoqing-zhou
as code owners
Signed-off-by: Polina Binder <[email protected]>
Signed-off-by: Polina Binder <[email protected]>
Signed-off-by: Polina Binder <[email protected]>
Signed-off-by: Polina Binder <[email protected]>
Signed-off-by: Polina Binder <[email protected]>
sub-packages/bionemo-fw/tests/bionemo/fw/test_dependency_graph.py
Outdated
Show resolved
Hide resolved
sub-packages/bionemo-fw/tests/bionemo/fw/test_dependency_graph.py
Outdated
Show resolved
Hide resolved
sub-packages/bionemo-fw/tests/bionemo/fw/test_dependency_graph.py
Outdated
Show resolved
Hide resolved
sub-packages/bionemo-fw/tests/bionemo/fw/test_dependency_graph.py
Outdated
Show resolved
Hide resolved
sub-packages/bionemo-fw/tests/bionemo/fw/test_dependency_graph.py
Outdated
Show resolved
Hide resolved
sub-packages/bionemo-fw/tests/bionemo/fw/test_dependency_graph.py
Outdated
Show resolved
Hide resolved
sub-packages/bionemo-fw/tests/bionemo/fw/test_dependency_graph.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Polina Binder <[email protected]>
Signed-off-by: Polina Binder <[email protected]>
docs/docs/assets/images/sub_package_graphs/dependency_file_imports.png
Outdated
Show resolved
Hide resolved
Signed-off-by: Polina Binder <[email protected]>
This generates a dependency graph between the bionemo sub-packages. Additionally, this will check that the pyproject.toml files agree with what's in the source files. This will also parse the source files to make sure that dependencies are correct between the bionemo sub-packages.