[ENH] text overrun on plot_critical_difference #3098

[Lucky-Lodhi2004]

Reference Issues/PRs

Fixes #3098 (critical difference diagram)

What does this implement/fix? Explain your changes.

The plot_critical_difference method generated diagrams where estimator labels were not visible properly and rank values intersected with the lines. for example

before :

The names of labels were very large, so i reduced size of names and scaled rank positions properly. Now label names will be visible properly and rank values will not intersect with the lines in most of the cases.

Few edge cases

If rank values are less than 0.5 or larger than 4.5 (let's Say for 5 models), then problems may arise.

for example :

Fixing these edge cases may require shifting labels to different places as compared to original design. I will work on this if maintainers want.

Does your contribution introduce a new dependency? If yes, which one?

No external dependencies are needed.

Any other comments?

not much to say.

PR checklist

For all contributions

• I've added myself to the list of contributors. Alternatively, you can use the @all-contributors bot to do this for you after the PR has been merged.
• [x ] The PR title starts with either [ENH], [MNT], [DOC], [BUG], [REF], [DEP] or [GOV] indicating whether the PR topic is related to enhancement, maintenance, documentation, bugs, refactoring, deprecation or governance.

For new estimators and functions

• I've added the estimator/function to the online API documentation.
• (OPTIONAL) I've added myself as a __maintainer__ at the top of relevant files and want to be contacted regarding its maintenance. Unmaintained files may be removed. This is for the full file, and you should not add yourself if you are just making minor changes or do not want to help maintain its contents.

For developers with write access

• (OPTIONAL) I've updated aeon's CODEOWNERS to receive notifications about future changes to these files.

[aeon-actions-bot]

Thank you for contributing to aeon

I have added the following labels to this PR based on the title: [ enhancement ].
I have added the following labels to this PR based on the changes made: [ clustering, visualisation ]. Feel free to change these if they do not properly represent the PR.

The Checks tab will show the status of our automated tests. You can click on individual test runs in the tab or "Details" in the panel below to see more information if there is a failure.

If our pre-commit code quality check fails, any trivial fixes will automatically be pushed to your PR unless it is a draft.

Don't hesitate to ask questions on the aeon Slack channel if you have any.

PR CI actions

These checkboxes will add labels to enable/disable CI functionality for this PR. This may not take effect immediately, and a new commit may be required to run the new configuration.

• Run pre-commit checks for all files
• Run mypy typecheck tests
• Run all pytest tests and configurations
• Run all notebook example tests
• Run numba-disabled codecov tests
• Stop automatic pre-commit fixes (always disabled for drafts)
• Disable numba cache loading
• Regenerate expected results for testing
• Push an empty commit to re-run CI checks

[baraline]

Thanks, any chance you could make an exemple to illustrate the edge case you are talking about ?

[Lucky-Lodhi2004]

Thanks, any chance you could make an exemple to illustrate the edge case you are talking about ?

Thanks for response.
I meant when rank values are too close to the ends of scale, (less than 0.5 and more than 4.5, then rank values may intersect with the lines), for example :

for handling this I would have to apply a checking condition and handle it for overlapping state. Somewhat like :

if ( linepos_x  < textarea + some_offset ) : 
    rank_xpos = linepos - some_other_offset
    rank_ypos = chei - some_other_offset

if ( linepos_x  > textarea + scale_width - some_offset) :
    rank_xpos = linepos + some_other_offset
    rank_ypos = chei - some_other_offset

this will make rank values be drawn here :

But for this I would have to add checking conditions and handling code here, thus changing structure of this particular function called plot_critical_difference

if reverse:
            _line(
                [
                    (_rankpos(ordered_avg_ranks[i]), cline),
                    (_rankpos(ordered_avg_ranks[i]), chei),
                    (textspace + scalewidth + 0.1, chei),
                ],
                linewidth=linewidth,
                color=colours[i],
            )
            _text(  # labels left side.
                textspace + scalewidth + 0.2,
                chei,
                ordered_labels[i],
                ha="left",
                va="center",
                size=9,
                color=colours[i],
            )
            _text(  # ranks left side.
                textspace + scalewidth - 0.15,
                chei - 0.075,
                format(ordered_avg_ranks[i], ".4f"),
                ha="left",
                va="center",
                size=7,
                color=colours[i],
            )

That's why I asked from maintainers whether I should do it.

[baraline]

I don't mind changing the function structure if it fixes the issue. If you have an immediate fix for it you can add it to the PR. Otherwise, we can go with this and raise a new issue for this particular problem so you have time to work on it.

[Lucky-Lodhi2004]

I don't mind changing the function structure if it fixes the issue. If you have an immediate fix for it you can add it to the PR. Otherwise, we can go with this and raise a new issue for this particular problem so you have time to work on it.

I need some time for further fixes. So yes, at this point, we should go with this.

If someone raise an issue about edge cases in 1-2 days I will make PR. Otherwise I will raise issue and submit PR myself.

But at this point, we should go with this.

Thanks for response.

[baraline]

Reviewing the issue and the code a bit deeper, both of the issues you identify are linked, and simply changing the padding values and size of the labels just postpone the problem to further "close to the edge" cases. This is not a general fix, it just works for this particular case.

We need to fix the origin issue, which is that the size of the horizontal line has of each estimator has no minimum width. To do that, I would recommend using a min_hline_width value (0.5 should be good) and define a line_end variable inside both of the for loop at line 313 at 173 such as :

if reverse:
    line_end = max(
          textspace + scalewidth + 0.1,
          _rankpos(ordered_avg_ranks[i]) + min_hline_width,
      )
    ...
else:
    line_end = min(
          textspace - 0.1,
          _rankpos(ordered_avg_ranks[i]) - min_hline_width,
      )

and for the second loop

if reverse:
    line_end = min(
        textspace - 0.1,
        _rankpos(ordered_avg_ranks[i]) - min_hline_width,
    )
    ...
else:
    line_end = max(
        textspace + scalewidth + 0.1,
        _rankpos(ordered_avg_ranks[i]) + min_hline_width,
    )

And use this when using the _line function, replacing the third argument (textspace + scalewidth + 0.1) by (line_end, chei) and similarly the first argument of the _text function. (Some minor adjustments might be needed on the padding values (-0.2, +0.4 etc.) that are added on the _text calls, i'd recommend trying without any padding first)

This should ensure that the figure is correctly built independently of the value of the textspace argument I think. Concerning the labels size, this should ideally be a parameter of the method, but we can skip that for now.