Refactor classes #8

zerofivefiveseven · 2022-07-21T15:00:07Z

wrote some methods to conversions.h
new structure
new conversions method
new base class
refactor calc methods

Add class interface

EDIT testcase. Now we read color image

codestyle fixes

create calc func instead of getkeypoints&getdescriptors delete most of private fields and now them scope is just culc function

# Conflicts: # src/feature_lib.cpp

TimPushkin

Looks great overall, but descriptors should be represented by a vector<vector<uint8_t>> because each key point has an assiciated array of uint8_ts.

include/feature_lib.h

examples/test.cpp

TimPushkin · 2022-07-21T16:17:11Z

Also, from the architectural aspect every class and struct in feature_lib.h should've been placed into a separate file. But as far as I know, Scapix has problems with porting structs from different headers, so we have to place everything together.

include/feature_lib.h

codestyle fixes add getters setters methods refactor constructor method refactor class variables storing now they store in baseclass edit example

TimPushkin

Use auto where possible if it doesn't make the code less clear. You can read more about when to use auto here.
Let's rename test.cpp to detection_example.cpp.

include/feature_lib.h

src/CMakeLists.txt

src/conversions.h

examples/test.cpp

remove setters refactor matto2dvec and renamed it edit codestyle & docstrings remove refactor mask storing

TimPushkin

You can still add auto in a lot of places

examples/detection_example.cpp

include/feature_lib.h

src/conversions.h

src/feature_lib.cpp

edit codestyle

TimPushkin

You got the setters wrong again :(

examples/detection_example.cpp

include/feature_lib.h

src/feature_lib.cpp

edit setters again edit docstring initialize width and height in header

TimPushkin

Other than this small detail, looks good to me

src/feature_lib.cpp

achains

Nice work, but it'd be better to remove relative import in examples

examples/detection_example.cpp

include/feature_lib.h

src/conversions.h

src/feature_lib.cpp

TimPushkin

Better move the opencv-contrib downloading and extracting into a separate block with the same check that the standard opencv has. Maybe even create a function for it because the same code is used twice.

BuildOpenCV.cmake

include/feature_lib.h

src/conversions.h

zerofivefiveseven · 2022-07-23T10:30:48Z

I did something with commits, and somehow strangely rolled back the changes in BuildOpenCV.cmake

TimPushkin · 2022-07-23T10:39:16Z

You did something wrong: you pushed the same commits and two lines related to the opencv-contrib changes are still there

zerofivefiveseven · 2022-07-23T10:51:53Z

rolled back changes in BuildOpenCV.cmake

change mat constructor in conversions.h edit constructor of base class now it use initializer list constructor remove default constructors now child class use initializer list constructor move definitions of base class to .cpp

zerofivefiveseven · 2022-07-23T11:41:04Z

rewrite history
add last requested correction

zerofivefiveseven added 8 commits July 20, 2022 12:03

Add SIFT implementation

d399e49

Add class interface

Add SIFT descriptor measure func

174fc8e

Some codestyle fixes

30d1fd3

ADD the method of converting color to grayscale to class constructor

4422205

EDIT testcase. Now we read color image

rename lib and associated header

6ab0b75

codestyle fixes

Add abstract class

7d18968

create calc func instead of getkeypoints&getdescriptors delete most of private fields and now them scope is just culc function

Add abstract class

b2c8421

create calc func instead of getkeypoints&getdescriptors delete most of private fields and now them scope is just culc function

Merge remote-tracking branch 'origin/cpp/dev' into cpp/dev

a12a506

# Conflicts: # src/feature_lib.cpp

zerofivefiveseven requested a review from TimPushkin July 21, 2022 15:00

TimPushkin requested changes Jul 21, 2022

View reviewed changes

TimPushkin reviewed Jul 21, 2022

View reviewed changes

include/feature_lib.h Show resolved Hide resolved

descriptor conversion method added

cbccec9

codestyle fixes add getters setters methods refactor constructor method refactor class variables storing now they store in baseclass edit example

zerofivefiveseven requested a review from TimPushkin July 22, 2022 11:15

TimPushkin requested changes Jul 22, 2022

View reviewed changes

new constructor

f2f6cd1

remove setters refactor matto2dvec and renamed it edit codestyle & docstrings remove refactor mask storing

zerofivefiveseven requested a review from TimPushkin July 22, 2022 13:47

edit codestyle

9c1cded

TimPushkin requested changes Jul 22, 2022

View reviewed changes

new setters which update sizes of mask matrix

56a4ac9

edit codestyle

zerofivefiveseven requested a review from TimPushkin July 22, 2022 15:28

TimPushkin requested changes Jul 22, 2022

View reviewed changes

edit codestyle

abba247

edit setters again edit docstring initialize width and height in header

zerofivefiveseven requested a review from TimPushkin July 22, 2022 16:31

TimPushkin requested changes Jul 22, 2022

View reviewed changes

src/feature_lib.cpp Outdated Show resolved Hide resolved

TimPushkin requested review from achains and anastasiia-kornilova July 22, 2022 16:38

achains reviewed Jul 22, 2022

View reviewed changes

examples/detection_example.cpp Outdated Show resolved Hide resolved

zerofivefiveseven requested review from achains and TimPushkin July 22, 2022 17:17

TimPushkin approved these changes Jul 22, 2022

View reviewed changes

achains approved these changes Jul 22, 2022

View reviewed changes

anastasiia-kornilova requested changes Jul 22, 2022

View reviewed changes

zerofivefiveseven mentioned this pull request Jul 22, 2022

Features is unacceptable. Should rename somehow #12

Closed

zerofivefiveseven requested a review from anastasiia-kornilova July 22, 2022 22:27

TimPushkin requested changes Jul 23, 2022

View reviewed changes

anastasiia-kornilova reviewed Jul 23, 2022

View reviewed changes

include/feature_lib.h Outdated Show resolved Hide resolved

src/conversions.h Outdated Show resolved Hide resolved

zerofivefiveseven requested review from anastasiia-kornilova and TimPushkin July 23, 2022 10:54

change cast type in conversions.h

ded8029

change mat constructor in conversions.h edit constructor of base class now it use initializer list constructor remove default constructors now child class use initializer list constructor move definitions of base class to .cpp

zerofivefiveseven force-pushed the cpp/dev branch from 8084996 to ded8029 Compare July 23, 2022 11:34

change Mat constructor of a base class field named mask

2c64f3e

anastasiia-kornilova approved these changes Jul 23, 2022

View reviewed changes

TimPushkin approved these changes Jul 23, 2022

View reviewed changes

zerofivefiveseven merged commit d335d9d into cpp/main Jul 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor classes #8

Refactor classes #8

zerofivefiveseven commented Jul 21, 2022

TimPushkin left a comment

TimPushkin commented Jul 21, 2022

TimPushkin left a comment •

edited

TimPushkin left a comment •

edited

TimPushkin left a comment

TimPushkin left a comment

achains left a comment

TimPushkin left a comment

zerofivefiveseven commented Jul 23, 2022

TimPushkin commented Jul 23, 2022

zerofivefiveseven commented Jul 23, 2022

zerofivefiveseven commented Jul 23, 2022

Refactor classes #8

Refactor classes #8

Conversation

zerofivefiveseven commented Jul 21, 2022

TimPushkin left a comment

Choose a reason for hiding this comment

TimPushkin commented Jul 21, 2022

TimPushkin left a comment • edited

Choose a reason for hiding this comment

TimPushkin left a comment • edited

Choose a reason for hiding this comment

TimPushkin left a comment

Choose a reason for hiding this comment

TimPushkin left a comment

Choose a reason for hiding this comment

achains left a comment

Choose a reason for hiding this comment

TimPushkin left a comment

Choose a reason for hiding this comment

zerofivefiveseven commented Jul 23, 2022

TimPushkin commented Jul 23, 2022

zerofivefiveseven commented Jul 23, 2022

zerofivefiveseven commented Jul 23, 2022

TimPushkin left a comment •

edited

TimPushkin left a comment •

edited