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

Hotfix/1.11.10 #46

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Hotfix/1.11.10 #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fcf92b
Merge branch 'hotfix/1.11.9'
danovaro Apr 2, 2024
b1843f0
added to-by support for stepranges
danovaro Apr 8, 2024
7fd6d6d
PR comments
danovaro Apr 10, 2024
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
2 changes: 1 addition & 1 deletion VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.11.9
1.11.10
3 changes: 1 addition & 2 deletions src/metkit/mars/StepRange.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ void StepRange::print(std::ostream& s) const
eckit::Time f{static_cast<long>(from_*3600.), true};
eckit::Time t{static_cast<long>(to_*3600.), true};

TimeUnit unit = std::min(maxUnit(f), maxUnit(t));
s << canonical(f, unit) << '-' << canonical(t, unit);
s << canonical(f) << '-' << canonical(t);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we no longer using unit here?

Copy link
Member Author

Choose a reason for hiding this comment

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

the initial request was to use the same unit for stepranges,
so 30m-1h ==> 30m-60m

with the to-by, we have horrible sequences:
0-6/to/3-9/by/30m ==> 0-6/30m-390m/1-7/90m-450m/2-8/150m-510m/3-9
with the proposed change we have:
0-6/to/3-9/by/30m ==> 0-6/30m-6h30m/1-7/1h30m-7h30m/2-8/2h30m-8h30m/3-9

}
}

Expand Down
9 changes: 9 additions & 0 deletions src/metkit/mars/StepRange.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ class StepRange {

operator std::string() const;

StepRange& operator+=(const eckit::Time& step) {
from_ += step/3600.;
to_ += step/3600.;
return *this;
}

bool operator==(const StepRange& other) const
{ return from_ == other.from_ && to_ == other.to_; }

Expand All @@ -69,6 +75,9 @@ class StepRange {
bool operator<(const StepRange& other) const
{ return (from_ != other.from_)?(from_<other.from_):(to_<other.to_); }

bool operator<=(const StepRange& other) const
{ return (from_ != other.from_)?(from_<=other.from_):(to_<=other.to_); }

bool operator>(const StepRange& other) const
{ return other < *this; }

Expand Down
25 changes: 14 additions & 11 deletions src/metkit/mars/TypeRange.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,14 @@ void TypeRange::print(std::ostream &out) const {
out << "TypeRange[name=" << name_ << "]";
}

bool TypeRange::expand(const MarsExpandContext& ctx, std::string& value) const {

StepRange TypeRange::parse(std::string& value) const {
eckit::Tokenizer parse("-");
std::vector<std::string> result;

parse(value, result);
switch (result.size()) {
case 1: {
value = StepRange(eckit::Time(result[0], true));
return true;
return StepRange(eckit::Time(result[0], true));
}
case 2: {
eckit::Time start = eckit::Time(result[0], true);
Expand All @@ -60,15 +58,20 @@ bool TypeRange::expand(const MarsExpandContext& ctx, std::string& value) const {
oss << name_ + ": initial value " << start << " cannot be greater that final value " << end;
throw eckit::BadValue(oss.str());
}
value = StepRange(start, end);
return true;
return StepRange(start, end);
}
default:
std::ostringstream oss;
oss << name_ + ": invalid value " << value << " " << result.size();
throw eckit::BadValue(oss.str());
}
return false;
}
}

bool TypeRange::expand(const MarsExpandContext& ctx, std::string& value) const {

value = parse(value);
return true;

}

void TypeRange::expand(const MarsExpandContext& ctx, std::vector<std::string>& values) const {
Expand All @@ -93,10 +96,10 @@ void TypeRange::expand(const MarsExpandContext& ctx, std::vector<std::string>& v
throw eckit::BadValue(oss.str());
}

eckit::Time from = eckit::Time(values[i - 1], true);
StepRange from = parse(values[i - 1]);
// unit = maxUnit(from);

eckit::Time to = eckit::Time(values[i + 1], true);
StepRange to = parse(values[i + 1]);
eckit::Time by = by_;

if (i+2 < values.size() && eckit::StringTools::lower(values[i + 2]) == "by") {
Expand All @@ -121,7 +124,7 @@ void TypeRange::expand(const MarsExpandContext& ctx, std::vector<std::string>& v
oss << name_ + ": 'by' value " << by << " must be a positive number";
throw eckit::BadValue(name_ + ": 'by' value must be a positive number");
}
eckit::Time j = from;
StepRange j = from;
j += by;
for (; j <= to; j += by) {
newval.emplace_back(StepRange(j));
Expand Down
5 changes: 5 additions & 0 deletions src/metkit/mars/TypeRange.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
namespace metkit {
namespace mars {

class StepRange;
//----------------------------------------------------------------------------------------------------------------------

class TypeRange : public Type {
Expand All @@ -36,6 +37,10 @@ class TypeRange : public Type {
virtual void expand(const MarsExpandContext& ctx,
std::vector<std::string>& values) const override;

StepRange parse(std::string& value) const;

private: // attributes

eckit::Time by_;

};
Expand Down
85 changes: 51 additions & 34 deletions tests/test_expand.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "metkit/mars/Type.h"

#include "eckit/testing/Test.h"
#include "eckit/utils/Tokenizer.h"

using namespace eckit::testing;

Expand Down Expand Up @@ -233,44 +234,60 @@ CASE( "test_metkit_expand_12_time" ) {
void stepThrows(std::vector<std::string> values) {
expandKeyThrows("step", values);
}
void step(std::vector<std::string> values, std::vector<std::string> expected) {
void step(std::string valuesStr, std::string expectedStr) {
eckit::Tokenizer parse("/");
std::vector<std::string> values;
std::vector<std::string> expected;

parse(valuesStr, values);
parse(expectedStr, expected);

expandKey("step", values, expected);
}

CASE( "test_metkit_expand_13_step" ) {
step({"0"}, {"0"});
step({"1"}, {"1"});
step({"24"}, {"24"});
step({"144"}, {"144"});
step({"012"}, {"12"});
step({"12:30"}, {"12h30m"});
step({"1:00"}, {"1"});
step({"1:0:0"}, {"1"});
step({"1h"}, {"1"});
step({"60m"}, {"1"});
step({"1h60m"}, {"2"});
step({"0:5"}, {"5m"});
step({"0:05"}, {"5m"});
step({"0:05:0"}, {"5m"});
step({"0:06"}, {"6m"});
step({"0:10"}, {"10m"});
step({"0:12"}, {"12m"});
step({"0:15"}, {"15m"});
step({"0:20"}, {"20m"});
step({"0:25"}, {"25m"});
step({"0:30"}, {"30m"});
step({"0:35"}, {"35m"});
step({"0:40"}, {"40m"});
step({"0:45"}, {"45m"});
step({"0:50"}, {"50m"});
step({"0:55"}, {"55m"});
step({"0-24"}, {"0-24"});
step({"0-24s"}, {"0s-24s"});
step({"0-120s"}, {"0m-2m"});
step({"0s-120m"}, {"0-2"});
step({"1-2"}, {"1-2"});
step({"30m-1"}, {"30m-60m"});

step("0", "0");
step("1", "1");
step("24", "24");
step("144", "144");
step("012", "12");
step("12:30", "12h30m");
step("1:00", "1");
step("1:0:0", "1");
step("1h", "1");
step("60m", "1");
step("1h60m", "2");
step("0:5", "5m");
step("0:05", "5m");
step("0:05:0", "5m");
step("0:06", "6m");
step("0:10", "10m");
step("0:12", "12m");
step("0:15", "15m");
step("0:20", "20m");
step("0:25", "25m");
step("0:30", "30m");
step("0:35", "35m");
step("0:40", "40m");
step("0:45", "45m");
step("0:50", "50m");
step("0:55", "55m");
step("0-24", "0-24");
step("0-24s", "0-24s");
step("0-120s", "0-2m");
step("0s-120m", "0-2");
step("1-2", "1-2");
step("30m-1", "30m-1");
step("30m-90m", "30m-1h30m");

step("0/to/24/by/3", "0/3/6/9/12/15/18/21/24");
step("12/to/48/by/12", "12/24/36/48");
step("12/to/47/by/12", "12/24/36");
step("0/to/6/by/30m", "0/30m/1/1h30m/2/2h30m/3/3h30m/4/4h30m/5/5h30m/6");
step("0-6/to/18-24/by/6", "0-6/6-12/12-18/18-24");
step("0-24/to/48-72/by/24", "0-24/24-48/48-72");
step("0/to/24/by/3/0-6/to/18-24/by/6", "0/3/6/9/12/15/18/21/24/0-6/6-12/12-18/18-24");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make sure there is a test here testing the ranges with the by syntax using e.g.

0-24/to/48-72/by/90m

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks, added!

step("0-24/to/24-48/by/90m", "0-24/1h30m-25h30m/3-27/4h30m-28h30m/6-30/7h30m-31h30m/9-33/10h30m-34h30m/12-36/13h30m-37h30m/15-39/16h30m-40h30m/18-42/19h30m-43h30m/21-45/22h30m-46h30m/24-48");
}


Expand Down
Loading