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

Change all occurrences of "method" to "algorithm" #6094

Closed
rlmill mannequin opened this issue May 20, 2009 · 41 comments
Closed

Change all occurrences of "method" to "algorithm" #6094

rlmill mannequin opened this issue May 20, 2009 · 41 comments

Comments

@rlmill
Copy link
Mannequin

rlmill mannequin commented May 20, 2009

In a discussion with David, we realized that he's been using "method" for "algorithm" in several places. This fix will follow up on #5701, and will probably depend on the patches there.

CC: @wdjoyner

Component: coding theory

Author: David Joyner, William Stein, Johan Sebastian Rosenkilde Nielsen

Reviewer: Rob Beezer, Robert Miller

Merged: sage-4.6.1.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/6094

@rlmill rlmill mannequin added this to the sage-4.6.1 milestone May 20, 2009
@rlmill rlmill mannequin added c: coding theory labels May 20, 2009
@rlmill rlmill mannequin self-assigned this May 20, 2009
@jasongrout
Copy link
Member

comment:1

"method" is not only easier to say, but it's easier to spell, and probably is more memorable to people...

@rlmill
Copy link
Mannequin Author

rlmill mannequin commented May 21, 2009

comment:2

Replying to @jasongrout:

"method" is not only easier to say, but it's easier to spell, and probably is more memorable to people...

It is also not the standard arg name. If you think method is better, take it to the mailing list.

@jasongrout
Copy link
Member

comment:3

okay, I didn't realize there was a standard. The weight of history changes the situation a bit.

@rlmill
Copy link
Mannequin Author

rlmill mannequin commented May 22, 2009

comment:4

Replying to @jasongrout:

okay, I didn't realize there was a standard...

Actually, neither did I. But when David and I did a search for "method=", most of what we found were places where he was using that name.

In fact, here is a complete (I think) list of the functions which use this (4.0.alpha0):

calculus/desolvers.py: eulers_method, eulers_method_2x2
coding/decoder.py: decode
coding/linear_code.py: decode, is_permutation_equivalent, permutation_automorphism_group, spectrum
finance/easter.py: easter
combinat/designs/block_design.py: ProjectiveGeometryDesign
combinat/designs/covering_design.py: __init__, method
combinat/designs/incidence_structures.py: dual_incidence_structure
groups/matrix_gps/matrix_group.py: as_permutation_group, module_composition_factors
schemes/elliptic_curves/padic_lseries.py: frobenius
matrix/matrix_double_dense.pyx: exp
matrix/matrix_modn_sparse.pyx: _rank_linbox
rings/polynomial/polynomial_compiled.pyx: __init__

@wdjoyner
Copy link

comment:5

Thanks. I'll get to these when #5701 is applied unless someone says I should just create a patch based on the patches there.

@wdjoyner
Copy link

based on 4.0.rc0 and all patches in #5701

@wdjoyner
Copy link

comment:6

Attachment: trac_6094-method-vs-algorithm.patch.gz

The current patch passes sage -testall with guava removed and all patched on #5701 applied.

@jhpalmieri
Copy link
Member

comment:8

Fails to apply cleanly to Sage 4.0.1.

applying /home/palmieri/trac_6094-method-vs-algorithm.patch
patching file sage/coding/linear_code.py
Hunk #1 FAILED at 315
Hunk #2 FAILED at 341
Hunk #3 FAILED at 351
Hunk #5 FAILED at 1144
Hunk #6 FAILED at 1165
Hunk #11 FAILED at 1636
Hunk #12 FAILED at 1651
Hunk #13 FAILED at 1668
Hunk #15 FAILED at 1758
Hunk #22 FAILED at 2179
10 out of 23 hunks FAILED -- saving rejects to file sage/coding/linear_code.py.rej
patching file sage/combinat/designs/block_design.py
Hunk #2 FAILED at 89
1 out of 3 hunks FAILED -- saving rejects to file sage/combinat/designs/block_design.py.rej
abort: patch failed to apply

@jhpalmieri jhpalmieri changed the title Change all occurrences of "method" to "algorithm" [needs rebase] Change all occurrences of "method" to "algorithm" Jun 9, 2009
@wdjoyner
Copy link

comment:9

I don't know how to rebase. Can someone point me to a reference? I might be able to do it this weekend.

@williamstein
Copy link
Contributor

Attachment: trac_6094-rebased_against_4.3.1.rc0.patch.gz

I rebased this against 4.3.1.rc0.

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Jan 18, 2010

Reviewer: Rob Beezer

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Jan 18, 2010

comment:11

Passes all tests. There are more instances of the use of method= in coding/code_bounds.py which will have a patch at #7971. Positive review.

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Jan 18, 2010

Author: David Joyner, William Stein

@rbeezer rbeezer mannequin changed the title [needs rebase] Change all occurrences of "method" to "algorithm" Change all occurrences of "method" to "algorithm" Jan 18, 2010
@williamstein
Copy link
Contributor

comment:13

It was pointed out by people in our status reports session that this patch violates our deprecation policy. Yuck. I.e., technically we should do

def f(algorithm='default', method=None):
    if method is not None:
        deprecation('...')
        algorithm = method

basically everywhere, then remove them all in a year. Hmmmm....

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Jan 18, 2010

comment:14

I've sent this back to "needs work" to await deprecation warnings. With more to do, include the changes in ccoding/code_bounds.py here as part of this ticket (see #7971).

@rbeezer rbeezer mannequin added s: needs work and removed s: positive review labels Jan 18, 2010
@williamstein
Copy link
Contributor

comment:15

I wonder if it could be done with a decorator?

@deprecate_method
def f(..., method="foo"):
    ...

@johanrosenkilde
Copy link
Contributor

comment:26

By the way, this patch contains all changes which were at one point suggested to be contained in Trac #7971. That trac can therefore be closed/invalidated.

@rlmill
Copy link
Mannequin Author

rlmill mannequin commented Nov 9, 2010

Changed reviewer from Rob Beezer to Rob Beezer, Robert Miller

@rlmill
Copy link
Mannequin Author

rlmill mannequin commented Nov 9, 2010

comment:27

If I apply #6094, #9907, #9919, and #10107 together on top of sage-4.6, all long tests pass. The code looks good.

I have also closed #7971.

@rlmill rlmill mannequin added s: positive review and removed s: needs review labels Nov 9, 2010
@jdemeyer
Copy link

Work Issues: rebase

@jdemeyer
Copy link

comment:28

The patch needs to be rebased to sage-4.6.1.alpha0:

patching file doc/en/constructions/linear_codes.rst
patching file sage/calculus/desolvers.py
Hunk #1 succeeded at 61 (offset 3 lines).
Hunk #2 succeeded at 829 (offset 117 lines).
Hunk #3 succeeded at 854 (offset 117 lines).
Hunk #4 succeeded at 886 (offset 117 lines).
Hunk #5 succeeded at 900 (offset 117 lines).
Hunk #6 succeeded at 938 (offset 117 lines).
Hunk #7 succeeded at 994 (offset 117 lines).
patching file sage/coding/code_bounds.py
patching file sage/coding/decoder.py
patching file sage/coding/linear_code.py
Hunk #1 succeeded at 202 (offset 2 lines).
Hunk #2 succeeded at 322 (offset 2 lines).
Hunk #3 succeeded at 359 (offset 2 lines).
Hunk #4 succeeded at 369 (offset 2 lines).
Hunk #5 succeeded at 1115 (offset 2 lines).
Hunk #6 succeeded at 1143 (offset 2 lines).
Hunk #7 succeeded at 1164 (offset 2 lines).
Hunk #8 succeeded at 1174 (offset 2 lines).
Hunk #9 succeeded at 1535 (offset 2 lines).
Hunk #10 succeeded at 1556 (offset 2 lines).
Hunk #11 succeeded at 1580 (offset 2 lines).
Hunk #12 succeeded at 1716 (offset 2 lines).
Hunk #13 succeeded at 1731 (offset 2 lines).
Hunk #14 succeeded at 1745 (offset 2 lines).
Hunk #15 succeeded at 1775 (offset 2 lines).
Hunk #16 succeeded at 1814 (offset 2 lines).
Hunk #17 succeeded at 1871 (offset 2 lines).
Hunk #18 succeeded at 1891 (offset 2 lines).
Hunk #19 succeeded at 1905 (offset 2 lines).
Hunk #20 succeeded at 1937 (offset 2 lines).
Hunk #21 succeeded at 1954 (offset 2 lines).
Hunk #22 succeeded at 2289 (offset 13 lines).
Hunk #23 succeeded at 2307 (offset 13 lines).
Hunk #24 succeeded at 2326 (offset 13 lines).
Hunk #25 succeeded at 2387 (offset 13 lines).
patching file sage/combinat/designs/block_design.py
patching file sage/combinat/designs/incidence_structures.py
patching file sage/finance/easter.py
patching file sage/groups/matrix_gps/matrix_group.py
patching file sage/interfaces/ecm.py
patching file sage/matrix/matrix_double_dense.pyx
Hunk #4 FAILED at 1524.
Hunk #5 succeeded at 1544 (offset 1 line).
Hunk #6 succeeded at 1567 (offset 1 line).
1 out of 6 hunks FAILED -- saving rejects to file sage/matrix/matrix_double_dense.pyx.rej
patching file sage/matrix/matrix_modn_sparse.pyx
Hunk #2 succeeded at 778 with fuzz 2.
Hunk #3 succeeded at 791 with fuzz 2.
patching file sage/rings/polynomial/polynomial_compiled.pyx
patching file sage/schemes/elliptic_curves/padic_lseries.py
patching file sage/symbolic/expression.pyx
Hunk #2 succeeded at 6443 (offset 745 lines).
Hunk #3 succeeded at 6452 (offset 745 lines).
Hunk #4 succeeded at 6504 (offset 745 lines).
Hunk #5 succeeded at 6642 (offset 745 lines).
Hunk #6 succeeded at 6651 (offset 745 lines).
Hunk #7 succeeded at 6698 (offset 745 lines).
Hunk #8 succeeded at 6718 (offset 745 lines).
Hunk #9 succeeded at 6734 (offset 745 lines).
Hunk #10 succeeded at 6760 (offset 745 lines).
Hunk #11 succeeded at 6827 (offset 745 lines).
Hunk #12 succeeded at 6864 (offset 745 lines).

@johanrosenkilde
Copy link
Contributor

Rebased for 4.6.1.alpha0. Still requires patches for #9919 and #9907 (in that order)

@johanrosenkilde
Copy link
Contributor

comment:29

Attachment: trac_6094-4.6.1.alpha0.with_deprecation.patch.gz

@johanrosenkilde
Copy link
Contributor

comment:30

Robert Miller already gave the green light to the old version, so the review is only for the minor change in sage.matrix.matrix_double_dense made for rebasing.

@rlmill
Copy link
Mannequin Author

rlmill mannequin commented Nov 26, 2010

Changed work issues from rebase to none

@jdemeyer
Copy link

jdemeyer commented Dec 2, 2010

Merged: sage-4.6.1.alpha3

@jdemeyer
Copy link

Changed author from David Joyner, William Stein, Johan S. R. Nielsen to David Joyner, William Stein, Johan Sebastian Rosenkilde Nielsen

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

No branches or pull requests

6 participants