Add namespace on image C++ codebase #3312
Conversation
datumbox
force-pushed
the
refactor/image
branch
2 times, most recently
from
8424391 to
8450858
Compare
datumbox
force-pushed
the
refactor/image
branch
from
8450858 to
7136251
Compare
datumbox
changed the title
Codecov Report
@@ Coverage Diff @@
## master #3312 +/- ##
=======================================
Coverage 73.93% 73.93%
=======================================
Files 104 104
Lines 9594 9594
Branches 1531 1531
=======================================
Hits 7093 7093
Misses 2024 2024
Partials 477 477 Continue to review full report at Codecov.
|
datumbox
force-pushed
the
refactor/image
branch
from
1690a5f to
22925ff
Compare
| namespace vision { | ||
| namespace image { | ||
|
|
||
| C10_EXPORT torch::Tensor decode_jpeg( |
There was a problem hiding this comment.
Here Github fails to track the right file renames and thinks I moved png to jpeg and vice versa.
| namespace vision { | ||
| namespace image { | ||
|
|
||
| C10_EXPORT torch::Tensor decode_png( |
There was a problem hiding this comment.
Again Github fails to figure out which file was moved to what here.
| namespace vision { | ||
| namespace image { | ||
|
|
||
| C10_EXPORT torch::Tensor encode_jpeg( |
There was a problem hiding this comment.
Git fails to track the old file.
|
|
||
| #include <torch/types.h> | ||
|
|
||
| C10_EXPORT torch::Tensor encodeJPEG(const torch::Tensor& data, int64_t quality); |
There was a problem hiding this comment.
Git fails to track the move to the new file...
| namespace image { | ||
| namespace detail { | ||
|
|
||
| #if JPEG_FOUND |
There was a problem hiding this comment.
Looks like we have two #if JPEG_FOUND in this file, but there doesn't seem to be any code not within JPEG_FOUND. Should we have instead a single #if JPEG_FOUND?
There was a problem hiding this comment.
I intentionally use two because I wanted to put the headers on top and then actually have the namespace vision::image::detail because I open it from a different file. This way even if it's empty the code will still work. I think I can refactor this to make it work with one if statement.
| @@ -0,0 +1,13 @@ | |||
| #pragma once | |||
|
|
|||
| #include <torch/types.h> | |||
There was a problem hiding this comment.
I need to start using a smart editor. I would have just #include <torch/torch.h> and brought everything in :-)
There was a problem hiding this comment.
Unfortunately that's not a smart editor feature but I wish they had that functionality too.
Summary: * Moving jpegcommon inside cpu implementation * Adding namespaces on image and moving private methods to anonymous. * Fixing headers. * Renaming public image methods to match the ones on python. * Refactoring to remove the double ifs in common_jpeg.h Reviewed By: fmassa Differential Revision: D26197738 fbshipit-source-id: a9d3ca2d8a248504fbe4bdfda94b6cbdda2270f1
Aligning the C++ codebase in
imagewith the rest of the library, similar to #3311:vision::imagenamespace.