Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added support for 3 bands float32 tiff images #1138

Merged
merged 3 commits into from
Dec 7, 2022
Merged

Added support for 3 bands float32 tiff images #1138

merged 3 commits into from
Dec 7, 2022

Conversation

AlexandreBrown
Copy link
Contributor

@AlexandreBrown AlexandreBrown commented Aug 25, 2022 •
edited
Loading

Fixes #1135
Fixes #1162

@ai-fast-track
Copy link
Collaborator

Thanks a lot for your contribution, Alex!

@potipot
Copy link
Contributor

potipot commented Dec 6, 2022

Any idea why the tests are not passing? Maybe we could collaboratively sort this out, I noticed it's been quite a while since they passed even on master.

@AlexandreBrown
Copy link
Contributor Author

AlexandreBrown commented Dec 6, 2022 •
edited
Loading

@potipot I haven't figure out why the tests on master are not passing (which is reflected in this MR since the tests fail).
I noticied that most if not all the failing tests seem to be related to name 'create_coco_eval' is not defined but would need to investigate more (this method was not changed or altered in this MR, we get this from master).
At least I was able to confirm that the classes affected by this MR and their test files that I modified and added all pass.

@ai-fast-track We are currently in a weird spot because our team environment currently has to depend on my personal fork branch (not ideal!) therefore we would like to get it merged asap.
Do you think we can create a separate issue for tests and then have it looked independently of this MR since the issue was already present?

I created a new issue so that we can collaborate on fixing the unit tests on master #1162

@AlexandreBrown
Removed circular dependency
818b7a7 
- Bug introduced by airctic#1132
- Credits goes to https://github.com/FraPochetti for finding the issue
@codecov
Copy link

codecov bot commented Dec 6, 2022 •
edited
Loading

Codecov Report

Merging #1138 (818b7a7) into master (72d30c9) will decrease coverage by 0.47%.
The diff coverage is 66.66%.

❗ Current head 818b7a7 differs from pull request most recent head 893ccd0. Consider uploading reports for the commit 893ccd0 to get more accurate results

@@            Coverage Diff             @@
##           master    #1138      +/-   ##
==========================================
- Coverage   85.81%   85.33%   -0.48%     
==========================================
  Files         305      288      -17     
  Lines        6710     6533     -177     
==========================================
- Hits         5758     5575     -183     
- Misses        952      958       +6
Flag Coverage Δ
unittests 85.33% <66.66%> (-0.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
icevision/data/convert_records_to_coco_style.py 77.16% <18.18%> (-20.71%) ⬇️
icevision/models/inference.py 77.27% <71.42%> (+1.88%) ⬆️
icevision/utils/imageio.py 81.00% <97.05%> (+1.27%) ⬆️
icevision/core/mask.py 81.01% <100.00%> (ø)
icevision/core/record_components.py 81.41% <100.00%> (-0.17%) ⬇️
icevision/data/convert_records_to_fo.py 85.89% <100.00%> (ø)
icevision/models/inference_sahi.py 94.89% <100.00%> (+0.05%) ⬆️
...sion/tfms/albumentations/albumentations_adapter.py 93.33% <100.00%> (ø)
...sion/tfms/albumentations/albumentations_helpers.py 97.87% <100.00%> (ø)
... and 17 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fstroth
Copy link
Contributor

fstroth commented Dec 6, 2022

@ai-fast-track @potipot We fixed a circular import that is currently in the lib. We should merge this asap, everything looks good to me. Can one of you have a second look to be sure, I would merge then.

potipot
potipot reviewed Dec 7, 2022
View reviewed changes
tests/utils/test_imageio.py Outdated Show resolved Hide resolved
potipot
potipot approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@potipot potipot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with comments

@potipot
Copy link
Contributor

potipot commented Dec 7, 2022

@AlexandreBrown do you remember what was the go-to approach in icevision? Does the author merge his PR or is it the reviewers responsibility?

@AlexandreBrown
Copy link
Contributor Author

AlexandreBrown commented Dec 7, 2022 •
edited
Loading

@AlexandreBrown do you remember what was the go-to approach in icevision? Does the author merge his PR or is it the reviewers responsibility?

In my experience the reviewers merged the PR (basing my response from my previous PR on IceVision).

@potipot potipot merged commit ffc0815 into airctic:master Dec 7, 2022
dnth added a commit to dnth/icevision that referenced this pull request Mar 6, 2024
@dnth @ai-fast-track
@AlexandreBrown @potipot @boscacci @lgvaz @fcakyon
Sync (#2)
25d5ca7 
* install torchtext==0.11.0 (airctic#1119)

* install icevision from master (as default) (airctic#1121)

* Feature/1124 Added Pytorch Lightning Test support for all models (airctic#1125)

* feature/1124 Added Pytorch Lightning Test support for mmdet

* feature/1124 Added Pytorch Lightning Test support for efficientdet

* feature/1124 Added Pytorch Lightning Test support for torchvision

* feature/1124 Added Pytorch Lightning Test support for yolov5

* feature/1124 Renamed shared evaluation method for mmdet

* feature/1124 Renamed shared evaluation method for efficientdet

* feature/1124 Renamed shared evaluation method for torchvision

* feature/1124 Renamed shared evaluation method for yolov5

* feature/1124 Added test for PL test step of efficientdet

* feature/1124 Added test for PL test step of mmdet

* feature/1124 Added test for PL test step of torchvision models

* feature/1124 Added test for PL test step of yolov5

* feature/1124 Updated object detection getting started guide to add PL test

* feature/1124 Updated environment.yml to fix icevision version not found

* feature/1124 Added unit tests for efficientdet lightning model adapter

* feature/1124 Added unit tests for mmdet lightning model adapter

* feature/1124 Added unit tests for torchvision lightning model adapter

* feature/1124 Added unit tests for yolov5 lightning model adapter

* feature/1124 Added unit tests for fastai unet lightning model adapter

* feature/1124 Fixed memory consumption in PL tests

* doc/1116 Added direction on how to import IceVision without wildcard import (airctic#1127)

* Update tests to use newer pytorch lightning (airctic#1133)

* force install pl>=1.7.0

* replace deprecated arguments

* bump minimal pl version from 1.4.6 to 1.5.0

* Export preds to coco annotations (airctic#1132)

* Add a function to export preds to COCO annotations

* Downsample example images to save space; add optional metadata parameter

* Separate 'info' field from the rest of 'addl_info' as a parameter

* Fix unsightly notebook outputs

* Fix slightly errant docstring. TODO: Add type hints

* Black formatting

* Clean up cluttered inference notebook output (airctic#1137)

Co-authored-by: Ubuntu <[email protected]>

* Make COCO exports remember original img resolutions (airctic#1142)

* Make COCO exports remember original img resolutions

* black formatting

* updates sahi dependency

* add support for sahi>=0.11 (airctic#1156)

* add support for sahi>0.11

* pin sahi version

* Added support for 3 bands float32 tiff images (airctic#1138)

* Added support for 3 bands float32 tiff images

* Removed circular dependency
- Bug introduced by airctic#1132
- Credits goes to https://github.com/FraPochetti for finding the issue

* Unit tests micro refactor

---------

Co-authored-by: AI Fast Track <[email protected]>
Co-authored-by: Alexandre Brown <[email protected]>
Co-authored-by: Paweł Potrykus <[email protected]>
Co-authored-by: Robert Boscacci <[email protected]>
Co-authored-by: Ubuntu <[email protected]>
Co-authored-by: Lucas Vazquez <[email protected]>
Co-authored-by: fatih <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potipot potipot potipot approved these changes

@lgvaz lgvaz Awaiting requested review from lgvaz

@FraPochetti FraPochetti Awaiting requested review from FraPochetti

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix unit tests failing on CI and/or local env Cannot open tif images when using COCO parser
4 participants
@AlexandreBrown @ai-fast-track @potipot @fstroth