Project

General

Profile

Feature #10974

[PATCH] Remove methods which has suffix `!`(sin!, cos!…) from CMath

Added by gogotanaka (Kazuki Tanaka) over 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:<unknown>]

Description

Hi, now I’d like to remove methods with !( sin!, cos!…) from CMath, for the following reasons.

  • wanna keep CMath minimal. CMath#sin should be superset of CMath#sin! or Math#sin is enough.

  • rdoc for CMath seems broken, CMath#sin! isn't alias for CMath#sin

thanks, gogo.

remove_methods_from_cmath.patch View (7.89 KB) gogotanaka (Kazuki Tanaka), 03/15/2015 08:27 AM

deprecate_CMath_methods.patch View (6.4 KB) gogotanaka (Kazuki Tanaka), 06/11/2015 06:31 AM

Associated revisions

Revision 52715
Added by ngoto (Naohisa Goto) over 1 year ago

  • lib/cmath.rb: methods which has suffix '!' are now deprecated. Re-apply r52469 made by Kazuki Tanaka, with fixing bug about mathn.rb compatibility. [Feature #10974]

Revision 52715
Added by ngoto (Naohisa Goto) over 1 year ago

  • lib/cmath.rb: methods which has suffix '!' are now deprecated. Re-apply r52469 made by Kazuki Tanaka, with fixing bug about mathn.rb compatibility. [Feature #10974]

Revision 52715
Added by ngoto (Naohisa Goto) over 1 year ago

  • lib/cmath.rb: methods which has suffix '!' are now deprecated. Re-apply r52469 made by Kazuki Tanaka, with fixing bug about mathn.rb compatibility. [Feature #10974]

History

#1 [ruby-core:69427] Updated by matz (Yukihiro Matsumoto) about 2 years ago

I am OK with this change. Does anyone else have opinon on this?
E.g. compatibility issue?

Matz.

#2 [ruby-core:69434] Updated by normalperson (Eric Wong) about 2 years ago

matz@ruby-lang.org wrote:

I am OK with this change. Does anyone else have opinon on this?
E.g. compatibility issue?

I don't use CMath, but I suggest a deprecation period since it appears
this is a public API. We should never break public API without adequate
warning.

Maybe:
1. deprecate at 2.3.0
2. remove when 2.3 branch is EOL for backports/releases

#3 [ruby-core:69521] Updated by gogotanaka (Kazuki Tanaka) about 2 years ago

Eric Wong wrote:

I don't use CMath, but I suggest a deprecation period since it appears
this is a public API. We should never break public API without adequate
warning.

Maybe:
1. deprecate at 2.3.0
2. remove when 2.3 branch is EOL for backports/releases

Thanks for comment! I suppose these methods(Math.cos!, sin!..) got public by accident, but your point dose really make sense.
Here is a patch to deprecate these methods.

I'll commit it in a while.

#4 [ruby-core:71371] Updated by gogotanaka (Kazuki Tanaka) over 1 year ago

  • Subject changed from [PATCH] Remove methods with `!`(sin!, cos!…) from CMath to [PATCH] Remove methods which has suffix `!`(sin!, cos!…) from CMath

#5 [ruby-core:71372] Updated by Hanmac (Hans Mackowiak) over 1 year ago

i know this ticket got closed by the change of gogo,
but i want to ask (maybe in a different ticket later)
if its would be ok to have a deprecate_methods function like the deprecate_constant method in core?

#11588 might be similar to what i want

#6 [ruby-core:71608] Updated by ngoto (Naohisa Goto) over 1 year ago

The infinite loop observed during RubySpec test in r52469
(http://rubyci.s3.amazonaws.com/ubuntu1510/ruby-trunk/log/20151106T153002Z.fail.html.gz )
is caused by the following code in mathn.rb.

unless defined?(Math.exp!)
  Object.instance_eval{remove_const :Math}
  Math = CMath # :nodoc:
end

#7 Updated by ngoto (Naohisa Goto) over 1 year ago

  • Status changed from Open to Closed

Applied in changeset r52715.


  • lib/cmath.rb: methods which has suffix '!' are now deprecated. Re-apply r52469 made by Kazuki Tanaka, with fixing bug about mathn.rb compatibility. [Feature #10974]

Also available in: Atom PDF