axondeepseg / axon-detection

Exploration of object detection applied to the AxonDeepSeg project
1 stars 0 forks source link

Train-val-test split script for YOLO and RetinaNet #10

Closed MurielleMardenli200 closed 2 months ago

MurielleMardenli200 commented 2 months ago
hermancollin commented 2 months ago

Guys... thank you for presenting your work in this PR but why are there 5000+ files changed :sob: I can't even view the files changed it makes my browser crash. Github says you added 2.9 million lines of code in this PR alone.

jcohenadad commented 2 months ago

@MurielleMardenli200 @kliem84 Also: please make sure to not upload binary data (eg: https://github.com/axondeepseg/axon-detection/pull/10/files#diff-e2e767adbf8d16a59e9f23a53aaac79f98b9923109ebccaffe9e13ebb2f9af2b)

git is made to version track code, not data. If you'd like to share data, you can upload them in issues/PR. In some cases they can be in the git history but pls ask @hermancollin before doing so.

We won't be able to merge this branch, it will be too difficult to cleanup

mathieuboudreau commented 2 months ago

Guys... thank you for presenting your work in this PR but why are there 5000+ files changed 😭 I can't even view the files changed it makes my browser crash. Github says you added 2.9 million lines of code in this PR alone.

Hahahaha, this is the best PR I've encountered.

I'll start reviewing each line today, /s

No but seriously, I think there was simply an accidental git add . that added the venv folder (as well as maybe some submodules?), a git squash + .gitignore could possible fix it, or simply closing this PR and making a new branch would be quicker.

mathieuboudreau commented 2 months ago

But, a .gitignore file (eg. https://github.com/axondeepseg/axondeepseg/blob/master/.gitignore ) is strongly recommended to avoid this type of problem. Overall not a major accident, but this branch should very much be deleted to avoid the binary issue julien mentionned bloating future git clones

mathieuboudreau commented 2 months ago

Oh also, as a fellow git add . user, I'd recommend always doing a git status check afterwards to see which files/folders are going to be committed prior to git commit in order to catch this type of accident, which I've done myself in the past

hermancollin commented 2 months ago

Yep Murielle actually added a .gitignore but I think at this point the files were already in the git history. So bottom line is this PR should be closed, the branch deleted and we should start over again! @MurielleMardenli200 or @kliem84 I'll let you deal with this.

mathieuboudreau commented 2 months ago

@MurielleMardenli200 @kliem84 Also: please make sure to not upload binary data (eg: https://github.com/axondeepseg/axon-detection/pull/10/files#diff-e2e767adbf8d16a59e9f23a53aaac79f98b9923109ebccaffe9e13ebb2f9af2b)

git is made to version track code, not data. If you'd like to share data, you can upload them in issues/PR. In some cases they can be in the git history but pls ask @hermancollin before doing so.

We won't be able to merge this branch, it will be too difficult to cleanup

To add on to this, binary files can also be safely uploaded on github when making a new version release https://github.com/axondeepseg/axon-detection/releases . I think it still counts towards data usage limits, but it's not going to be version tracked through git

MurielleMardenli200 commented 2 months ago

@MurielleMardenli200 @kliem84 Also: please make sure to not upload binary data (eg: https://github.com/axondeepseg/axon-detection/pull/10/files#diff-e2e767adbf8d16a59e9f23a53aaac79f98b9923109ebccaffe9e13ebb2f9af2b) git is made to version track code, not data. If you'd like to share data, you can upload them in issues/PR. In some cases they can be in the git history but pls ask @hermancollin before doing so. We won't be able to merge this branch, it will be too difficult to cleanup

To add on to this, binary files can also be safely uploaded on github when making a new version release https://github.com/axondeepseg/axon-detection/releases . I think it still counts towards data usage limits, but it's not going to be version tracked through git

Thank you for the information on the binary files! I think in this case it's just a small function update and we won't need to make a version release but I'll make a note of it for the next time.

And yes like @hermancollin said I'll close the branch PR, delete the branch to make a cleaner PR this time! 😅