Automated comment on GitHub Pull Requests#78
Conversation
|
This is a fancy comment |
Packing analysis reportAnalysis for packing results located at cellpack/tests/outputs/test_spheres/jitter Packing imageDistance analysisExpected minimum distance: 50.00 Possible errors:
Distance distribution: |
.gitignore
Outdated
| **/converted/* | ||
|
|
||
| # test outputs | ||
| cellpack/tests/outputs/ |
| "ingredient_key" : "ext_A", | ||
| "run_similarity_analysis": true, | ||
| "save_plots": true, | ||
| "max_plots_to_save": 1 |
There was a problem hiding this comment.
TODO: formalize the analysis config options and add default settings for all of them
cellpack/autopack/Analysis.py
Outdated
| interior_z z coordinates of reference particles | ||
| """ | ||
| from numpy import zeros, sqrt, where, pi, average, arange, histogram | ||
| from numpy import arange, average, histogram, pi, sqrt, where, zeros |
There was a problem hiding this comment.
[nit] - these imports should be at the top of the file
There was a problem hiding this comment.
Huh, looks like these were imported automatically. I'll check on this.
There was a problem hiding this comment.
Looks like these functions are leftover from the original codebase. Not sure if they're in use anymore. Might be worth going over them at some point to check their coverage.
cellpack/autopack/Analysis.py
Outdated
| interior_y y coordinates of reference particles | ||
| """ | ||
| from numpy import zeros, sqrt, where, pi, average, arange, histogram | ||
| from numpy import arange, average, histogram, pi, sqrt, where, zeros |
There was a problem hiding this comment.
same here, move imports to top of file
|
|
||
| return minimum_packed_distance | ||
|
|
||
| def create_report(self, report_options): |
There was a problem hiding this comment.
maybe for another PR, but this could be put in our "Writers" file as a more generic md writer
cellpack/autopack/Analysis.py
Outdated
| """ | ||
| Returns the minimum distance between packed objects | ||
| """ | ||
| minimum_packed_distance = 9999 |
There was a problem hiding this comment.
this could be initialized to the diagonal of the bounding box to be a more accurate starting point
There was a problem hiding this comment.
Initializing this to infinity since we don't know the size of the bounding box.
| if plot and two_d: | ||
| width = 1000.0 # should be the boundary here ? | ||
| fig = pyplot.figure() | ||
| width = self.env.get_size_of_bounding_box() |
There was a problem hiding this comment.
oh, this was causing problems before, thanks for fixing
There was a problem hiding this comment.
Doesn't fit the scope of this PR, but it's a small enough change that we can keep it.
* use infinity as reference starting distance
2 participants
This PR tests the automated comment functionality
Closes #35