Fix large numbers and add timestamp support #273
Conversation
|
Just a note, what seem like formatting issues are actually the result of your own |
There was a problem hiding this comment.
Thanks for this! :)
Fixes using large number for ticks
Why? Were the numbers outside the margins? What is the issue here?
and adds support for rendering timestamps
Are these two things related?
Just a note, what seem like formatting issues are actually the result of your own .clang-format file, for example some lines were changed because you have an 80 character column limit and you probably haven't touched these files since you added the column limit
That's fine. We don't reformat the whole thing whenever parameters change. We only reformat files as we change them.
| @@ -143,6 +143,8 @@ namespace matplot { | |||
|
|
|||
| std::string bars::data_string() { | |||
| std::stringstream ss; | |||
| ss.precision(5); | |||
There was a problem hiding this comment.
Were these generating some kind of gnuplot warning?
Shouldn't gnuplot understand any precision?
There was a problem hiding this comment.
The default stringstream precision prevented rendering of very small numbers
There was a problem hiding this comment.
Also I believe (these are some old changes I wanted to PR so it's been a while) gnuplot would start using scientific notation without that change
| if (terminal_ == "dumb") { | ||
| run_command("set terminal dumb " + num2str(position_[2]) + " " + | ||
| num2str(position_[3])); | ||
| } else { |
There was a problem hiding this comment.
else if (terminal_has_size_option(terminal_))?
There was a problem hiding this comment.
Okay wait this shouldn't be here lmao, but I do remember why I put this here, the dumb terminal is super funny and actually has a nonstandard way of setting the size, as you can see here, but it's not relevant to this PR
| if (terminal_has_size_option(terminal_)) { | ||
| run_command("set terminal " + terminal_ + " size " + | ||
| num2str(position_[2]) + "," + num2str(position_[3])); | ||
| if (terminal_ == "dumb") { |
There was a problem hiding this comment.
Sorry... How are these changes related to the PR?
| if (terminal_ == "dumb") { | ||
| run_command("set terminal dumb " + num2str(position_[2]) + " " + | ||
| num2str(position_[3])); | ||
| } else { |
| @@ -106,6 +107,9 @@ namespace matplot { | |||
| return *this; | |||
| } | |||
|
|
|||
| bool is_timestamp() const; | |||
There was a problem hiding this comment.
Is the timestamp related to the ss.precision(5)?
There was a problem hiding this comment.
timestamp(true) enables timestamp rendering, so ax->xtickformat ("%m/%d/%Y %H:%M:%S"); can be used for example. ss.precision() is relevant to large numbers
| @@ -154,6 +158,8 @@ namespace matplot { | |||
| float label_font_size_{11}; | |||
| color_array label_color_{0, 0, 0, 0}; | |||
|
|
|||
| bool is_timestamp_{false}; | |||
There was a problem hiding this comment.
Is that all? Do other libraries also work like that?
source/matplot/core/axis_type.cpp
Outdated
| if (is_timestamp_) { | ||
| char buff[20] = {0}; | ||
| time_t now = (time_t)tick_values_[i]; | ||
| strftime(buff, 20, tick_label_format_.c_str(), |
There was a problem hiding this comment.
Will the default tick_label_format_ work for timestamps?
There was a problem hiding this comment.
No, because strftime supports formatting the hour, minute, second, etc using a millisecond input. num2str does not do this
| @@ -46,6 +46,8 @@ namespace matplot { | |||
|
|
|||
| template <class T> std::string num2str(Arithmetic<T> num) { | |||
| std::ostringstream ss; | |||
| ss.precision(5); | |||
There was a problem hiding this comment.
I'm not sure I completely understand the motivation for precision(5) at other classes, but it doesn't look like a sensible default for this function.
For instance,
str(2.5433653543232154254214321541)gives me
'2.5433653543232153'in python.
|
Okay so the dumb terminal should be removed entirely (though you do explicitly support the dumb terminal so this is sorta a bugfix), and |
|
I should mention this explicitely as well, the issue I was referring to was large numbers being converted to scientific notation by default, which is somewhat annoying especially in the y axis |
|
|
|
Glad it was useful! Got a bit preoccupied on other code, because of the font issue I shelved this project |
Fixes using large number for ticks and adds support for rendering timestamps in, for example, xticks