Packager directory fix #507
Conversation
|
|
…o packager-directory-fix
There was a problem hiding this comment.
Thanks for the PR! Looks good overall. Couple of notes:
- Make sure lint passes (see my inline comment)
- Can you add a test for this functionality to test_python_packaging.py? You can look at the other tests in the containing folder to get an idea of how they work. (Feel free to comment here if you have questions)
Also just to note, the test output you posted in the PR description from ./build/test.sh is actually showing the output of the Java inference tests. The python and packaging tests are higher up in the output :)
Thanks again!
| #Ensures that neuropod_path is not a subdirectory of python_root | ||
| #Ensures neuropod_path and python_root are not the same directory | ||
| if os.path.samefile(neuropod_path,python_root) or os.path.realpath(neuropod_path).startswith( | ||
| os.path.realpath(python_root) + os.sep | ||
| ): | ||
| raise ValueError( | ||
| "`neuropod_path` cannot be a subdirectory of `python_root`" | ||
| "`neuropod_path` cannot be the same directory as `python_root' or a subdirectory of `python_root`." | ||
| ) |
There was a problem hiding this comment.
Have you run lint locally to check formatting and code style?
From https://neuropod.ai/docs/stable/developing/#lint-and-static-analysis:
To show all lint errors and warnings locally, you can run
./tools/lint.sh. To attempt to automatically fix any issues that can be automatically fixed, run./tools/autofix.sh.
There was a problem hiding this comment.
I have not run lint. The link to the lint description in the contributions guidelines and did not open to the page you added when i tried to open it.
|
@nemanjarajic did you intend to close this PR? |
|
@VivekPanyam sorry i'm still work on getting the hang of github. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #507 +/- ##
==========================================
+ Coverage 87.20% 87.27% +0.07%
==========================================
Files 111 111
Lines 7298 7301 +3
==========================================
+ Hits 6364 6372 +8
+ Misses 934 929 -5 ☔ View full report in Codecov by Sentry. |
Summary:
Added
to the if statement so that it would be triggered if neuropod_path and python_root are the same directory or neuropod_path is a subdirectory of python_root
Tests:
Using ./build/test.sh
INFO: Analyzed 35 targets (0 packages loaded, 0 targets configured).
INFO: Found 31 targets and 4 test targets...
INFO: Elapsed time: 1.243s, Critical Path: 0.99s
INFO: 5 processes: 1 internal, 4 processwrapper-sandbox.
INFO: Build completed successfully, 5 total actions
//neuropod/bindings/java:LibraryLoaderTest PASSED in 0.5s
WARNING: //neuropod/bindings/java:LibraryLoaderTest: Test execution time (0.5s excluding execution overhead) outside of range for MODERATE tests. Consider setting timeout="short" or size="small".
//neuropod/bindings/java:TFAdditionTest PASSED in 1.0s
WARNING: //neuropod/bindings/java:TFAdditionTest: Test execution time (1.0s excluding execution overhead) outside of range for MODERATE tests. Consider setting timeout="short" or size="small".
//neuropod/bindings/java:TensorSpecTest PASSED in 0.5s
WARNING: //neuropod/bindings/java:TensorSpecTest: Test execution time (0.5s excluding execution overhead) outside of range for MODERATE tests. Consider setting timeout="short" or size="small".
//neuropod/bindings/java:TorchscriptAdditionTest PASSED in 1.0s
WARNING: //neuropod/bindings/java:TorchscriptAdditionTest: Test execution time (1.0s excluding execution overhead) outside of range for MODERATE tests. Consider setting timeout="short" or size="small".
INFO: Build completed successfully, 5 total actions