Skip to content

Fix wrong comment and add assertion#337

Merged
tolik518 merged 3 commits intotolik518:masterfrom
YichiZhang0613:fix_inconsistency
Mar 27, 2026
Merged

Fix wrong comment and add assertion#337
tolik518 merged 3 commits intotolik518:masterfrom
YichiZhang0613:fix_inconsistency

Conversation

@YichiZhang0613
Copy link
Copy Markdown
Contributor

The comment indicates function will not panic when x is finite. However, when x is not positive it will panic, so I fix the comment to be more complete. What's more, I add an assertion with proper error message.

Copy link
Copy Markdown
Collaborator

@Aras14HD Aras14HD left a comment

Choose a reason for hiding this comment

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

Thanks for the correction!

But, I'd rather implement those cases.

On zero it should probably return (0, 0).
And to cover negative numbers, it should just use the absolute for the logarithm.

If you want, you can try to do that. Otherwise I can also do it.

@YichiZhang0613
Copy link
Copy Markdown
Contributor Author

Thanks for the correction!

But, I'd rather implement those cases.

On zero it should probably return (0, 0). And to cover negative numbers, it should just use the absolute for the logarithm.

If you want, you can try to do that. Otherwise I can also do it.

OK!

Copy link
Copy Markdown
Collaborator

@Aras14HD Aras14HD left a comment

Choose a reason for hiding this comment

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

Looks good!

Again thanks for the contribution!

@tolik518
Copy link
Copy Markdown
Owner

Thanks to both of you!
I'll merge the change later today

@tolik518 tolik518 merged commit a808c7d into tolik518:master Mar 27, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants