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.

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

Updates example section in the Readme #48

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Trybnetic
Copy link

As outlined in #47, this PR updates the documentation in the readme. Specifically, I made the examples consistent in the sense that they are both using the fruits.txt dataset now and extended the descriptions of the examples a bit.

Changes introduced in this PR:

  • Consistent use of fruits.txt in the examples
  • Extended descriptions of the examples
  • Overview of the function and its parameters in the readme
  • Updated the CI status badge

I see some further changes that could be included in this PR:

  • Clean up (e.g. the no longer used PNGs, the deprecated arguments mentioned in the readme, etc.)
  • Longer explanation on the use of leftWeight and rightWeight

@Pierre-Sassoulas
Copy link
Owner

Hey @Trybnetic, is this still a work in progress ?

@Trybnetic
Copy link
Author

I actually struggled with explaining on the use of leftWeight and rightWeight, but I will try the next days to do the mentioned clean up and then this PR should be ready to be merged.

@Trybnetic Trybnetic marked this pull request as ready for review October 21, 2024 11:11
Copy link
Owner

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Thank you, made some suggestions, let me know what you think and let's merge this.

Comment on lines +124 to +128
However, not always you have or can have the data available in the format mentioned in
the previous example (e.g. if the dataset is too large). In this case, the weights
between the true and predicted labels can also be calculated beforehand and used to
create the sankey diagram. In this example, we continue to work with the data loaded
already in the previous example:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
However, not always you have or can have the data available in the format mentioned in
the previous example (e.g. if the dataset is too large). In this case, the weights
between the true and predicted labels can also be calculated beforehand and used to
create the sankey diagram. In this example, we continue to work with the data loaded
already in the previous example:
However, the data may not always be available in the format mentioned in
the previous example (for instance, if the dataset is too large). In such cases, the weights
between the true and predicted labels can be calculated in advance and used to
create the Sankey diagram. In this example, we will continue working with the data that was loaded
in the previous example:

Comment on lines +16 to +17
`pysankey` contains a simple expected/predicted dataset called `fruits.txt` which looks
the following:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
`pysankey` contains a simple expected/predicted dataset called `fruits.txt` which looks
the following:
`pysankey` contains a simple expected/predicted dataset called `fruits.txt` which looks
like the following:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants