-
Notifications
You must be signed in to change notification settings - Fork 18
Put the MorphRGrid
at the end of the morphing chain
#232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Note that only the The files changed should go away once #231 is merged. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #232 +/- ##
=======================================
Coverage 99.08% 99.08%
=======================================
Files 21 21
Lines 1087 1087
=======================================
Hits 1077 1077
Misses 10 10 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will merge this so you can move ahead, but please see comment and make an issue. Also, in common with things like np.all_cloae
etc, could we drop the "are" from the name and make it more like files_all_close
or something like that. We could also borrow the signature and pass through tolerances. Again, this can be done later, after beam time... Just make an issue now so we don't forget
tests/test_morphio.py
Outdated
@@ -50,6 +50,20 @@ def isfloat(s): | |||
return False | |||
|
|||
|
|||
def are_files_same(file1, file2): | |||
"""Assert that two files have (approximately) the same numerical |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks as if it is a copy paste of the one in the other PR. Can we put it into conftest. Py and make it available to all tests as a fixture?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is from the other PR. After merging with main, this change is no longer present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only needed in the io tests as that is the only test creating files. As such, no need to put into contest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, ok
Created #233 just in case we want to revisit names. |
See news (
config_order.rst
) comment for why this is done. Preparation formorphfuncx
andmorphfuncxy
.