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

port to c++11 #89

Merged
merged 3 commits into from
May 14, 2018
Merged

port to c++11 #89

merged 3 commits into from
May 14, 2018

Conversation

sandsmark
Copy link
Contributor

removes the mandatory boost dependency, only thing left is boost::python, which ideally would be replaced with pybind11 (helped dlib quite a bit when it did).

@ethzasl-jenkins
Copy link

Can one of the admins verify this patch?

@sandsmark
Copy link
Contributor Author

davisking/dlib#293 fwiw

@HannesSommer
Copy link
Collaborator

ok to test

Copy link
Collaborator

@HannesSommer HannesSommer left a comment

Choose a reason for hiding this comment

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

Thank you very much for your great contribution and the very interesting hint concerning pybind11!
LGTM except for the minor suggestions!
Would you like to integrate them? Otherwise I'll do that.

tests/helpers.h Outdated
timer():_start_time(high_resolution_clock::now()){ }
void restart() { _start_time = high_resolution_clock::now(); }
double elapsed() const // return elapsed time in seconds
{ return duration_cast<chrono::nanoseconds>(high_resolution_clock::now() - _start_time).count() / double(1000000000); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be possible to write this more concise as

return duration_cast<duration<double>>(high_resolution_clock::now() - _start_time).count();

tests/helpers.h Outdated
#else // _POSIX_TIMERS
#include <boost/timer.hpp>
#endif // _POSIX_TIMERS
timer():_start_time(high_resolution_clock::now()){ }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd make high_resolution_clock::now() a private static function, e.g. curTime like before to deduplicate the three occurrences.

tests/helpers.h Outdated
namespace boost
#include <chrono>
using std::chrono::high_resolution_clock;
using std::chrono::duration_cast;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather use explicit scoping or function level using below.

@sandsmark
Copy link
Contributor Author

Hi!

Yeah, I actually wanted to do what you suggested, but I don't have my laptop here, so feel free to do it. Otherwise I can do it when I get home. :-)

Copy link
Collaborator

@HannesSommer HannesSommer left a comment

Choose a reason for hiding this comment

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

Sorry, I got distracted from this. I've just put my suggested changes on top. Ready to merge from my side.

@HannesSommer HannesSommer merged commit 3b15ebc into norlab-ulaval:master May 14, 2018
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