Skip to content

Conversation

PProfizi
Copy link
Contributor

Issue #219, The code used the index from enumerating the nodes instead of using the index_to_id function when retrieving the scalar value associated with a node in Plotter.add_label().

@akaszynski
Copy link
Contributor

akaszynski commented Apr 14, 2022

Let's add a unit test here.

Also, when you get a chance, it would be great to add code coverage so we can tell if lines or tested or not.

@PProfizi PProfizi self-assigned this Apr 27, 2022
@PProfizi PProfizi added the bug Something isn't working label Apr 27, 2022
@PProfizi
Copy link
Contributor Author

I could not see how to test this properly as this is really about displaying the right value in the plot.
As for test coverage it is among the things I want to implement in our pipelines as soon as I have time. I'll open an improvement issue.

@PProfizi PProfizi closed this Apr 27, 2022
@PProfizi PProfizi reopened this Apr 27, 2022
@PProfizi PProfizi requested a review from cbellot000 April 27, 2022 07:58
@PProfizi PProfizi requested a review from rlagha May 11, 2022 15:15
@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #221 (fb867be) into master (01b4ec6) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
+ Coverage   85.45%   85.61%   +0.15%     
==========================================
  Files          52       52              
  Lines        5650     5650              
==========================================
+ Hits         4828     4837       +9     
+ Misses        822      813       -9     

@PProfizi PProfizi merged commit c1b763b into master May 24, 2022
@PProfizi PProfizi deleted the fix/node_labeling_in_plots branch May 24, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants