Opened 4 years ago

Closed 4 years ago

#19550 closed enhancement (fixed)

Parameter name 'method' -> 'algorithm'

Reported by: jmantysalo Owned by:
Priority: minor Milestone: sage-6.10
Component: graph theory Keywords:
Cc: ​jdemeyer, ncohen Merged in:
Authors: Jori Mäntysalo Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: d30d530 (Commits) Commit: d30d5303f3e7d10e643318a4d1fea91ba3c370c6
Dependencies: Stopgaps:

Description

This patch changes some method to algorithm in graphs. Also it corrects some too deep indentation in nested lists in the documentation.

See discussion about parameter name at https://groups.google.com/forum/#!topic/sage-devel/KSQZ3fzxWac

Change History (8)

comment:1 Changed 4 years ago by jmantysalo

  • Branch set to u/jmantysalo/edge_cut_etc

comment:2 Changed 4 years ago by git

  • Commit set to d30d5303f3e7d10e643318a4d1fea91ba3c370c6

Branch pushed to git repo; I updated commit sha1. New commits:

d30d530Stupid typo.

comment:3 follow-up: Changed 4 years ago by ncohen

what do you think of algorithm="igraph", where igraph is a graph library?

comment:4 in reply to: ↑ 3 Changed 4 years ago by jmantysalo

Replying to ncohen:

what do you think of algorithm="igraph", where igraph is a graph library?

That at least it is better to have only algorithm than both algorithm and method. There is two keywords that we can get rid of: method and impl.

Theoretically there is a big difference between algorithm and implementation. For example we could have a buggy implementation of Newton's method. But does Sage has any function that could be used like solve(f, algorithm='newton', implementation='buggylibrary')?

comment:5 follow-up: Changed 4 years ago by ncohen

  • Reviewers set to Nathann Cohen
  • Status changed from new to needs_review

I don't think that algorithm='igraph' is very proper either, but at least we will get some kind of simplification by using 'algorithm' indeed.

Good to go,

Nathann

comment:6 Changed 4 years ago by ncohen

  • Status changed from needs_review to positive_review

comment:7 in reply to: ↑ 5 Changed 4 years ago by jmantysalo

Replying to ncohen:

Good to go,

Thanks. (Actually this was intentionally not in needs_review; but now it has also passed all tests after running over night.)

comment:8 Changed 4 years ago by vbraun

  • Branch changed from u/jmantysalo/edge_cut_etc to d30d5303f3e7d10e643318a4d1fea91ba3c370c6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.