-
Notifications
You must be signed in to change notification settings - Fork 32
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
Release version 4.0.0 #587
Conversation
…fter IVADOMED transition (#539) * Delete files/functions we suspect will be no longer necessary after IVADOMED transition, see https://docs.google.com/document/d/1Ipd6_nK6I6CKlDI_KpkLXH_jY2C547W_ootalfKnFlU/edit?usp=sharing' * Restore visualize segmentation * Remove imports from files that have been deleted * Remove imports from files that have been deleted Co-authored-by: Mathieu Boudreau <mathieuboudreau@Mathieus-MacBook-Pro.local>
…entation. Remove FSLeyes (temporarily) (#546) * Remove dependencies: TF, Keras, and Albumentation * Remove TF import and tf reset graph calls in tests * Update environment file for compatibility with Mac M1 ARM current setup with MiniForge (upgrade to python > 3.7 and update scikit versions * Update run_tests.yaml * Loosen fsleyes req * Downgrade to Python 3.8 for IVADOMED-compatibility * Downgrade python in the testing environment too
* Add IVADOMED in the requirements * Add IVADOMED in environment files * Add test IVADOMED SEM model * Update download_model.py * Update download_model.py * Update model * Early implementation of segmentation wrapper * Update missing import * Fix syntax * Convert Path() object to str() * Fix typo in string * Fix full path * Remove hardoced output filename * Save merge mask * Add missing import * Cleanup * Cleanup * Cleanup * Cleanup * Cleanup * Cleanup * Debug * Debug * Add tests for segment module * Try implementing segment_image * Debug * Initial implementation of segment_image for IVADOMED wrapper * Initial implementation of segment_folders for ivadomed * Make necessary changes to test_segment_folders_runs_with_relative_path * Resolve failing tests * Resolve test * Implement CLI for ivadomed * Update IVADOMED pixel_size option syntax * Update ads plugin for IVADOMED compatibility * Change ivadomed installation to pip * Updated notebooks for IVADOMED; deleted training notebooks * Update SEM model * Fix broken link * Refactor * Fix typo * Change missed line * Debug * Fix * Add TEM model * Final fixes * Add TEM * Fix typo * Fix IVADOMED related folder name * Add TEM option * Update release versions * Fix TEM model * Update release * Fix model names * Add pytest ini file * Resolve warnings * Resolve warnings * Test upgrading versions * Try Windows fix * Add find links * Update test files link * Update test files version * Update release * Update release version * Add OM model * Minor fixes * Minor fixes * Update release version * Update release version * Update release version * Implement IVADOMED into the ADS plugin * Fix bug where if no gratio and myelin stats were not initialized * Update test files release versions * Change convention from giving default NaNs to gratio and myelin stats to 0, for more robust save/load behaviours * Update one test * Change OM to BF nomenclature * Minor cleanup. Add documentation * Update environment.yml * Update AxonDeepSeg/integrity_test.py Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com> * Update AxonDeepSeg/morphometrics/compute_morphometrics.py Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com> * Review changes * Convert from 0 to np.nans * Remove duplicate line * Remove unnecessary line * Minor fixes in notebooks * Change filename convention * Switch from is to == to remove warnings * Add default pixel unit * Workaround to testing saved and loaded morphometrics with NaNs * #547 updated the plugin file Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com> Co-authored-by: Stoyan I. Asenov <43790661+Stoyan-I-A@users.noreply.github.com>
@mariehbourget this branch is ready for review. |
Hmm the coverage dropped, but that may be because of all the tests we removed when we removed the training files. Let me look into it, I could increase the limit temporarily if that's the case. |
Thank you @mathieuboudreau, I should be able to review by the end of the week. Quick notes:
|
Looks like the cause was that we removed the tests for visualize.py, but we didn't remove the source file itself when we were doing cleanup in 3c903dc. I just deleted it |
Hmm, not sure why the deleted file is still being counted in the number of lines during Coveralls |
Looks good now with +5% 🚀 |
The training microscopy tutorial in ivadomed is now merged if we want to link to it in the documentation. |
When we're ready to merge this branch, I'll update the checks settings from Python 3.7 to 3.8 to make them pass (we moved to 3.8 in this PR). |
Being done in #588 |
Done in 61c59a3 |
Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com>
Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com>
Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com>
@mariehbourget I just accepted all suggestions, and fixed the bug in reading resolutions in commit d0abfaa |
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.
Thank you @mathieuboudreau!
I think everything is working as expected.
I left some minor comment/suggestions but approve the PR since they are small details.
Question: I saw that #592 and #584 are approved, do we want to integrate them before merging?
Co-authored-by: Armand <83031821+hermancollin@users.noreply.github.com>
Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com>
@mariehbourget I've merged the two PRs, accepted your suggestions, and updated the Changelog per your suggestion. If this is ready to merge, please give it one final approval! |
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.
Thanks @mathieuboudreau!
2 small things, I promise to approve after that! :)
Perfect! Done and done =) |
Co-authored-by: mariehbourget <54086142+mariehbourget@users.noreply.github.com>
Sorry, I clicked commit changed but it didn't work the first time! It should be there now.
--
Mathieu Boudreau, PhD
Research Fellow | Montreal Heart Institute
Software Developer | qMRLab and AxonDeepSeg
Lead Editor | MRM Highlights<http://linkedin.com/company/mrm-highlights/><https://www.linkedin.com/company/mrm-highlights/>
________________________________
From: mariehbourget ***@***.***>
Sent: Thursday, February 3, 2022 13:44
To: neuropoly/axondeepseg ***@***.***>
Cc: Mathieu Boudreau ***@***.***>; Mention ***@***.***>
Subject: Re: [neuropoly/axondeepseg] Release version 4.0.0 (PR #587)
@mariehbourget commented on this pull request.
________________________________
In AxonDeepSeg/segment.py<#587 (comment)>:
requiredName.add_argument('-i', '--imgpath', required=True, nargs='+', help='Path to the image to segment or path to the folder \n'+
'where the image(s) to segment is/are located.')
- ap.add_argument("-m", "--model", required=False, help='Folder where the model is located. \n'+
- 'The default SEM model path is: \n'+str(default_SEM_path)+'\n'+
- 'The default TEM model path is: \n'+str(default_TEM_path)+'\n'+
- 'The default OM model path is: \n'+str(model_seg_pns_bf_path)+'\n')
+ ap.add_argument("-m", "--model", required=False, help='Folder where the model is located, if different from the default model.')
ap.add_argument('-s', '--sizepixel', required=False, help='Pixel size of the image(s) to segment, in micrometers. \n'+
'If no pixel size is specified, a pixel_size_in_micrometer.txt \n'+
'file needs to be added to the image folder path. The pixel size \n'+
'in that file will be used for the segmentation.',
default=None)
ap.add_argument('-v', '--verbose', required=False, type=int, choices=list(range(0,4)), help='Verbosity level. \n'+
Hey @mathieuboudreau<https://github.com/mathieuboudreau>, I don't see this change, is it normal?
—
Reply to this email directly, view it on GitHub<#587 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAK25ZJEMZGCNPTNOBQUTODUZK5GFANCNFSM5LZGIOJQ>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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.
Thank you so much @mathieuboudreau for coordinating this project. 🚀
Version 4 is ready! 😀
Thank you for your very rigourous review @mariehbourget and all your suggestsions !! |
Checklist
Description
Release version 4.0.0 of AxonDeepSeg that integrated IVADOMED into the project and provides Mac M1 compatibility.
Linked issues
Resolves #523 #536