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

Improve documentation of mathematical functions #9994

Merged
merged 1 commit into from Dec 7, 2020

Conversation

HertzDevil
Copy link
Contributor

Adds better documentation to the functions from the Math module. Unfortunately, this means the module no longer uses macros to quickly generate those LibM wrappers.

Also changes the parameter names of some functions, for consistency with either similar functions or other overloads:

  • Math.atan2(value1, value2) => Math.atan2(y, x)
  • Math.log(numeric, base) => Math.log(value, base)
  • Math.besselj(value1, value2) => Math.besselj(order, value), ditto for .bessely
  • Math.ldexp(value1, value2) => Math.ldexp(value, exp)
  • Math.scalbn(value1, value2) => Math.scalbn(value, exp)
  • Math.exp(z : Complex) => Math.exp(value : Complex), ditto for the Complex overloads of .log, .log2, .log10, and .sqrt

This could break code that uses named arguments for these mathematical functions. They are probably very rare though.

@HertzDevil
Copy link
Contributor Author

Technically there could be proper deprecation for the old parameter names, e.g.

module Math
  @[Deprecated]
  def atan2(*, value1, value2)
    atan2(value1, value2)
  end

  @[Deprecated]
  def atan2(value1, *, value2)
    atan2(value1, value2)
  end
end

Is this necessary? How do we approach this kind of breaking changes in general?

@straight-shoota
Copy link
Member

Yeah, that would be the proper way. But I don't expect anyone would actually pass named arguments for these Math methods, especially considering they weren't very soundly named.

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

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

FWIW I would not consider this a breaking-change. I don't expect the named arguments of these methods to be used that way.

@straight-shoota straight-shoota added this to the 1.0.0 milestone Dec 7, 2020
@straight-shoota straight-shoota merged commit 4c835ff into crystal-lang:master Dec 7, 2020
@HertzDevil HertzDevil deleted the docs/math branch December 8, 2020 02:54
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.

None yet

3 participants