Conversation
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
==========================================
+ Coverage 87.60% 87.67% +0.07%
==========================================
Files 47 47
Lines 2146 2167 +21
==========================================
+ Hits 1880 1900 +20
- Misses 266 267 +1
Continue to review full report at Codecov.
|
|
It seems that the last pull request was not merged. I wonder if the current pull request is acceptable or not. Looking for the positive response. Thanks! |
martinjrobins
left a comment
martinjrobins
left a comment
There was a problem hiding this comment.
Hi @zhongjingjogy. The previous PR was still in progress, so I'm not sure why you closed it! The new line styles look good, just a few things to tidy up see comments below
| } | ||
|
|
||
| // get the number of maximum char length of yticks | ||
| int max_ytick_char_len() const { return m_max_ytick_char_len; } |
There was a problem hiding this comment.
m_max_ytick_char_len depends on if the drawing code is run yet or not. please remove this method and keep this value purely private to the class
| int max_ytick_char_len() const { return m_max_ytick_char_len; } | ||
|
|
||
| // get default font size | ||
| float font_size() const { return m_style.font_size(); } |
There was a problem hiding this comment.
remove this too as is not needed.
| float font_size() const { return m_style.font_size(); } | ||
|
|
||
| // get maximum char | ||
| std::string max_ytick_char() const { return m_max_ytick_char; } |
|
|
||
| backend.text_align(ALIGN_RIGHT | ALIGN_MIDDLE); | ||
|
|
||
| size_t max_char_len = this->max_ytick_char_len(); |
There was a problem hiding this comment.
| size_t max_char_len = this->max_ytick_char_len(); | |
| size_t max_char_len = 0; |
|
|
||
| if (strlen(buffer) > max_char_len) { | ||
| max_char_len = strlen(buffer); | ||
| m_max_ytick_char = std::string(buffer); |
There was a problem hiding this comment.
think m_max_ytick_char is not used? If so then remove
| Vector<int, 2> get_ticks() const { return {m_nx_ticks, m_ny_ticks}; } | ||
|
|
||
| /// get the number of maximum yticks length (in pixels) | ||
| int max_ytick_pixels() const { |
There was a problem hiding this comment.
This calculation should really be done by the Style class, but since the calculation is still not correct please put it inline in Axis::draw_common_ylabel along with a TODO comment
|
|
||
|
|
||
| TEST_CASE("ylabel position: case 1", "[axis]") { | ||
| std::vector<double> predicted_data = { |
There was a problem hiding this comment.
rename these vectors x and y and use these below rather than creating another pair of vectors
2 participants
Try to plot with dashed lines.
