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

Feature/sub hourly #85

Merged
merged 8 commits into from
Dec 8, 2023
Merged

Feature/sub hourly #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5e63c45
parsing Time with units
danovaro Sep 5, 2023
9a34c67
Time parsing - added support for fraction of an hour
danovaro Sep 27, 2023
ded272c
Include cmath
joobog Sep 28, 2023
66302b1
added support for negative time/step
danovaro Oct 20, 2023
f9dcdf1
cleanup
danovaro Oct 27, 2023
1aa8aea
Merge branch 'develop' into feature/subHourly
danovaro Nov 18, 2023
c041bfa
fix merge
danovaro Nov 18, 2023
66acba4
addressed PR comments
danovaro Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/eckit/thread/ThreadPool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ class ThreadPool : private NonCopyable {
const std::string& name() const { return name_; }
void error(const std::string&);

bool done();
void wait();
bool done();
void resize(size_t);
Expand Down
173 changes: 96 additions & 77 deletions src/eckit/types/Time.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
* does it submit to any jurisdiction.
*/

#include <regex>
#include <cmath>

#include "eckit/eckit.h"

#include "eckit/persist/DumpLoad.h"
Copy link
Member

Choose a reason for hiding this comment

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

Remove #include "eckit/utils/Tokenizer.h"

#include "eckit/types/Time.h"
#include "eckit/utils/Hash.h"
#include "eckit/utils/Tokenizer.h"
#include "eckit/utils/Translator.h"

namespace eckit {

Expand All @@ -27,90 +29,108 @@ inline void printTime(std::ostream& s, long n) {
s << n;
}

Time::Time(long seconds) :
seconds_(seconds) {
if (seconds >= 86400 || seconds < 0) {
std::string msg = "Time in seconds must be positive and cannot exceed 86400, seconds: ";
Translator<long, std::string> t;
msg += t(seconds);
Time::Time(long seconds, bool extended) :
seconds_(static_cast<Second>(seconds)) {
if ((seconds >= 86400 && !extended) || seconds < 0) {
std::string msg = "Time in seconds must be positive and less than 86400 seconds (24h): ";
msg += std::to_string(seconds);
throw BadTime(msg);
}
}

Time::Time(const std::string& s) {
Tokenizer parse(":");
std::vector<std::string> result;

parse(s, result);

long hh = 0, mm = 0, ss = 0;
bool err = false;
long t = atol(s.c_str());

switch (result.size()) {
case 1:
// hh or hhmm or hhmmss
switch (s.length()) {
case 2:
hh = t;
break;
case 4:
hh = t / 100;
mm = t % 100;
break;
case 6:
hh = t / 10000;
mm = (t % 10000) / 100;
ss = (t % 10000) % 100;
break;
default:
err = true;
break;
Time::Time(const std::string& s, bool extended) {
long ss = 0;
long mm = 0;
long hh = 0;
long dd = 0;
std::smatch m;

if (std::regex_match (s, m, std::regex("^-?[0-9]+$"))) { // only digits
long t = std::stol(s);
int sign = (s[0] == '-' ? 1 : 0);
if (extended || s.length() <= 2+sign) { // cases: h, hh, (or hhh..h for step parsing)
hh = t;
} else {
if (s.length() <= 4+sign) { // cases: hmm, hhmm
hh = t / 100;
mm = t % 100;
} else { // cases: hmmss, hhmmss
hh = t / 10000;
mm = (t / 100) % 100;
ss = t % 100;
}
break;

case 2:
// hh:mm
err = result[0].length() != 2 || result[1].length() != 2;

hh = atol(result[0].c_str());
mm = atol(result[1].c_str());

break;

case 3:
// hh:mm:ss

err = result[0].length() != 2 || result[1].length() != 2 || result[2].length() != 2;

hh = atol(result[0].c_str());
mm = atol(result[1].c_str());
ss = atol(result[2].c_str());

break;

default:
err = true;
break;
}
}

if (err) {
throw BadTime(std::string("Invalid time ") + s);
else {
if (std::regex_match (s, m, std::regex("^-?[0-9]*\\.[0-9]+$"))) { // floating point (hours)
long sec = std::round(std::stod(s)*3600);
hh = sec/3600;
sec -= hh*3600;
mm = sec/60;
sec -= mm*60;
ss = sec;
}
else {
if (std::regex_match (s, m, std::regex("^([0-9]+):([0-5]?[0-9])(:[0-5]?[0-9])?$"))) {
for (int i=1; i<m.size(); i++) {
if (m[i].matched) {
switch (i) {
case 1: hh = std::stol(m[i].str()); break;
case 2: mm = std::stol(m[i].str()); break;
case 3: std::string aux = m[i].str();
aux.erase(0,1);
ss = std::stol(aux); break;
}
}
}
}
else {
if (std::regex_match (s, m, std::regex("^-?([0-9]+[dD])?([0-9]+[hH])?([0-9]+[mM])?([0-9]+[sS])?$"))) {
for (int i=1; i<m.size(); i++) {
if (m[i].matched) {
std::string aux = m[i].str();
aux.pop_back();
long t = std::stol(aux);
switch (i) {
case 1: dd = t; break;
case 2: hh = t; break;
case 3: mm = t; break;
case 4: ss = t;
}
}
}
ss += 60 * (mm + 60 * (hh + 24 * dd));
if (s[0] == '-') {
ss = -ss;
}
dd = ss / 86400;
hh = (ss / 3600) % 24;
mm = (ss / 60) % 60;
ss = ss % 60;
} else {
std::string msg = "Wrong input for time: ";
msg += s;
throw BadTime(msg);
}
}
}
}

if (hh >= 24 || mm >= 60 || ss >= 60 || hh < 0 || mm < 0 || ss < 0) {
if (mm >= 60 || ss >= 60 || (!extended && (hh >= 24 || dd > 0 || hh < 0 || mm < 0 || ss < 0))) {
std::string msg = "Wrong input for time: ";
Translator<long, std::string> t;
msg += t(hh);
if (dd>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 isn't consistent with the check above, for ss<86400. (eg, line 35 but there are more). If the class isn't to support days, and it also doesn't have and accessor for it, this should actually fail (or, remove the checks and support it)

Copy link
Member

Choose a reason for hiding this comment

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

Also, this printing of time as string is repeated in multiple places -- ideally, these places should be replaced with a operator std::string (that you already have)

msg += std::to_string(dd);
msg += " days ";
}
msg += std::to_string(hh);
msg += " hours ";
msg += t(mm);
msg += std::to_string(mm);
msg += " minutes ";
msg += t(ss);
msg += std::to_string(ss);
msg += " seconds";
throw BadTime(msg);
}

seconds_ = hh * 3600 + mm * 60 + ss;
seconds_ = dd * 86400 + hh * 3600 + mm * 60 + ss;
}

Time::operator std::string() const {
Expand All @@ -127,16 +147,15 @@ Time& Time::operator=(const Time& other) {
return *this;
}

Time::Time(long hh, long mm, long ss) :
Time::Time(long hh, long mm, long ss, bool extended) :
seconds_(hh * 3600 + mm * 60 + ss) {
if (hh >= 24 || mm >= 60 || ss >= 60 || hh < 0 || mm < 0 || ss < 0) {
if (mm >= 60 || ss >= 60 || (!extended && (hh >= 24 || hh < 0 || mm < 0 || ss < 0))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

IS there a reason we can't don't support 61 minutes, or 61 seconds, and lroll them into larger time units, given that we do that with hours here?

std::string msg = "Wrong input for time: ";
Translator<long, std::string> t;
msg += t(hh);
msg += std::to_string(hh);
msg += " hours ";
msg += t(mm);
msg += std::to_string(mm);
msg += " minutes ";
msg += t(ss);
msg += std::to_string(ss);
msg += " seconds";
throw BadTime(msg);
}
Expand Down
8 changes: 4 additions & 4 deletions src/eckit/types/Time.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ class DumpLoad;
class Bless;
class Hash;

typedef double Second;
using Second = double;

//----------------------------------------------------------------------------------------------------------------------

class Time {
Copy link
Member

Choose a reason for hiding this comment

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

Modernise
typedef double Second
to
using Second = double;


public: // methods
Time(long, long, long);
Time(long seconds = 0);
Time(const std::string&);
Time(long hours, long minutes, long seconds, bool extended = false);
Time(long seconds = 0, bool extended = false);
Time(const std::string& time, bool extended = false);

#include "eckit/types/Time.b"

Expand Down
4 changes: 4 additions & 0 deletions tests/types/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ ecbuild_add_test( TARGET eckit_test_types_floatcompare
SOURCES test_floatcompare.cc
LIBS eckit )

ecbuild_add_test( TARGET eckit_test_types_time
SOURCES test_time.cc
LIBS eckit )

ecbuild_add_test( TARGET eckit_test_types_uuid
SOURCES test_uuid.cc
LIBS eckit )
Expand Down
Loading
Loading