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

Adding the option to print values in the print action #63

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Adding the option to print values in the print action #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 20 additions & 1 deletion src/multio/action/print/Print.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ namespace multio::action {
Print::Print(const ComponentConfiguration& compConf) : ChainedAction(compConf) {
stream_ = compConf.parsedConfig().getString("stream", "info");
onlyFields_ = compConf.parsedConfig().getBool("only-fields", false);
includeValues_ = compConf.parsedConfig().getBool("include-data-values",false);
numberOfValues_ = compConf.parsedConfig().getInt("value-count",0);
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a start-index option (default to 0) to indicate at which index to start printing? I think this would be especially useful if the data starts with missing values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we know from which index missing values begin/end?

Copy link
Member

Choose a reason for hiding this comment

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

I guess you can look at the bitmap and count. We could automate this and accept this option to be set to "first-non-missing", but that requires a bit more work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would probably rather just have a mechanism to print missing values, i.e. write the string 'missing' or 'n/a' instead of a numeric value.


if (stream_ == "info") {
os_ = &eckit::Log::info();
Expand All @@ -45,7 +47,24 @@ void Print::executeImpl(message::Message msg) {
if (!prefix_.empty()) {
(*os_) << prefix_ << ": ";
}
(*os_) << msg << std::endl;
(*os_) << msg << "\n";
if(includeValues_) {
const auto* data = static_cast<const double*>(msg.payload().data());
const auto* end = data + (msg.payload().size() / sizeof(double));
Copy link
Member

Choose a reason for hiding this comment

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

I think our data can also be in 32 bits float instead of 64 bits double. In that case we should check the precision key and handle floats slightly different from doubles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah you're right. That was a hack that I did because I knew it was a double field that came in. We should discuss if we want to have this in at all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I think this would only be useful for tiny payloads. I guess there may not be harm including it, but the code will need a bit of tightening up beforehand. Apart from the precision support, we may want to make sure that we always use reference to ostream and a bit more 'modern' C++ for loops.


if (!data || data == end) {
throw eckit::SeriousBug{
"Payload is empty: Cannot print data: " + msg.metadata().toString(), Here()};
}

// Print the payload data
int count = 0;
for (auto it = data; (it != end) && count<numberOfValues_; ++it) {
(*os_) << *it << " ";
++count;
}
(*os_) << std::endl;
}
}
executeNext(std::move(msg));
}
Expand Down
2 changes: 2 additions & 0 deletions src/multio/action/print/Print.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ class Print : public ChainedAction {
void print(std::ostream& os) const override;

bool onlyFields_;
bool includeValues_;
std::string stream_;
int numberOfValues_;

std::ostream* os_;
std::string prefix_;
Expand Down
Loading