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.

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

Suvorov DM/accuracy verification #115

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

suvorovDm-1
Copy link
Collaborator

I added a file with the opening of all photos in the directory.

Copy link

codecov bot commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.87%. Comparing base (2a183a8) to head (98e4d38).
Report is 19 commits behind head on main.

❗ Current head 98e4d38 differs from pull request most recent head e604520. Consider uploading reports for the commit e604520 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #115      +/-   ##
==========================================
+ Coverage   78.44%   83.87%   +5.42%     
==========================================
  Files          13       17       +4     
  Lines         348      552     +204     
  Branches      166      280     +114     
==========================================
+ Hits          273      463     +190     
- Misses         62       71       +9     
- Partials       13       18       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -0,0 +1,23 @@
#include <opencv2/opencv.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'opencv2/opencv.hpp' file not found [clang-diagnostic-error]

#include <opencv2/opencv.hpp>
         ^

@aobolensk
Copy link
Member

Please, do not attach binaries here (I mean images by themselves)

Comment on lines 20 to 21
if (t1.get_shape().count() != t2.get_shape().count()) return false;
if (t1.get_type() != t2.get_type()) return false;
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather throw an error in this case, cause it is just invalid, not meaning that these tensors are not equal

#include <filesystem>

int main() {
std::string directory = "./photos";
Copy link
Member

Choose a reason for hiding this comment

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

Accept directory as cmd line argument

Comment on lines 14 to 15
cv::imshow("Image", image);
cv::waitKey(0);
Copy link
Member

Choose a reason for hiding this comment

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

This will show the images in a sequential order and block the execution. It is better not to do this. Our future application will not do that, actually.

What you need to do at this stage: read the content of the image, convert to tensor (cv::Mat -> Tensor, use our tensor class) and do nothing with this tensor (for now)

int main() {
std::string directory = "./photos";

for (const auto& entry : std::filesystem::directory_iterator(directory)) {
Copy link
Member

Choose a reason for hiding this comment

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

namespace fs = std::filesystem;

I suggest using this, it is a common widely-used way to simplify std::filesystem related code

#include <iostream>
#include <filesystem>

int main() {
Copy link
Member

Choose a reason for hiding this comment

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

please create help message for users

#include <filesystem>

int main() {
std::string image_directory = "./photos";
Copy link
Member

Choose a reason for hiding this comment

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

all paths need to be got from command line arguments

std::string image_directory = "./photos";

std::vector<cv::String> image_paths;
cv::glob(image_directory + "*.jpg", image_paths);
Copy link
Member

Choose a reason for hiding this comment

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

opencv can read extensions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants