Skip to content

burn python2.x bridges more #12948

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions benchmarks/bench_20newsgroups.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@

print("20 newsgroups")
print("=============")
print("X_train.shape = {0}".format(X_train.shape))
print("X_train.format = {0}".format(X_train.format))
print("X_train.dtype = {0}".format(X_train.dtype))
print("X_train density = {0}"
print("X_train.shape = {}".format(X_train.shape))
print("X_train.format = {}".format(X_train.format))
print("X_train.dtype = {}".format(X_train.dtype))
print("X_train density = {}"
"".format(X_train.nnz / np.product(X_train.shape)))
print("y_train {0}".format(y_train.shape))
print("X_test {0}".format(X_test.shape))
print("X_test.format = {0}".format(X_test.format))
print("X_test.dtype = {0}".format(X_test.dtype))
print("y_test {0}".format(y_test.shape))
print("y_train {}".format(y_train.shape))
print("X_test {}".format(X_test.shape))
print("X_test.format = {}".format(X_test.format))
print("X_test.dtype = {}".format(X_test.dtype))
print("y_test {}".format(y_test.shape))
Copy link
Member

@rth rth Jan 10, 2019

Choose a reason for hiding this comment

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

All changes of the form

- "something {0}".format(x)
+ "something {}".format(x)

don't really bring anything and should be reverted IMO. It's one character less, for the cost of creating merge conflicts with all the open PRs.

Copy link
Author

Choose a reason for hiding this comment

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

they do offer an improvement and were added in python2.7 -- numbering format specifiers is human-error-prone operation

Copy link
Member

Choose a reason for hiding this comment

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

Yes and it's good to use them going forward, but adding merge conflicts for this seems not useful.

Copy link
Author

Choose a reason for hiding this comment

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

I think you could say the same about using super() -- what's the hangup here?

Copy link
Member

Choose a reason for hiding this comment

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

Fair, that would be an argument to keep the super calls the same. But I do feel the new way makes them more readable. The "hangup" is that every code change brings maintenance costs with it and we're trying to balance the benefit of making the change with that cost.
This is the reason why we discourage changes that are largely cosmetic, like applying pep8.

Copy link
Member

Choose a reason for hiding this comment

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

I don't really have that strong an opinion, though...

print()

print("Classifier Training")
Expand Down
4 changes: 2 additions & 2 deletions benchmarks/bench_mnist.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,12 @@ def load_data(dtype=np.float32, order='F'):
print()
print("Classification performance:")
print("===========================")
print("{0: <24} {1: >10} {2: >11} {3: >12}"
print("{: <24} {: >10} {: >11} {: >12}"
"".format("Classifier ", "train-time", "test-time", "error-rate"))
print("-" * 60)
for name in sorted(args["classifiers"], key=error.get):

print("{0: <23} {1: >10.2f}s {2: >10.2f}s {3: >12.4f}"
print("{: <23} {: >10.2f}s {: >10.2f}s {: >12.4f}"
"".format(name, train_time[name], test_time[name], error[name]))

print()
2 changes: 1 addition & 1 deletion benchmarks/bench_multilabel_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def _plot(results, metrics, formats, title, x_ticks, x_label,
if args.plot is not None:
print('Displaying plot', file=sys.stderr)
title = ('Multilabel metrics with %s' %
', '.join('{0}={1}'.format(field, getattr(args, field))
', '.join('{}={}'.format(field, getattr(args, field))
for field in ['samples', 'classes', 'density']
if args.plot != field))
_plot(results, args.metrics, args.formats, title, steps, args.plot)
24 changes: 12 additions & 12 deletions benchmarks/bench_plot_neighbors.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ def barplot_neighbors(Nrange=2 ** np.arange(1, 11),

#------------------------------------------------------------
# varying N
N_results_build = dict([(alg, np.zeros(len(Nrange)))
for alg in algorithms])
N_results_query = dict([(alg, np.zeros(len(Nrange)))
for alg in algorithms])
N_results_build = {alg: np.zeros(len(Nrange))
for alg in algorithms}
N_results_query = {alg: np.zeros(len(Nrange))
for alg in algorithms}

for i, NN in enumerate(Nrange):
print("N = %i (%i out of %i)" % (NN, i + 1, len(Nrange)))
Expand All @@ -61,10 +61,10 @@ def barplot_neighbors(Nrange=2 ** np.arange(1, 11),

#------------------------------------------------------------
# varying D
D_results_build = dict([(alg, np.zeros(len(Drange)))
for alg in algorithms])
D_results_query = dict([(alg, np.zeros(len(Drange)))
for alg in algorithms])
D_results_build = {alg: np.zeros(len(Drange))
for alg in algorithms}
D_results_query = {alg: np.zeros(len(Drange))
for alg in algorithms}

for i, DD in enumerate(Drange):
print("D = %i (%i out of %i)" % (DD, i + 1, len(Drange)))
Expand All @@ -84,10 +84,10 @@ def barplot_neighbors(Nrange=2 ** np.arange(1, 11),

#------------------------------------------------------------
# varying k
k_results_build = dict([(alg, np.zeros(len(krange)))
for alg in algorithms])
k_results_query = dict([(alg, np.zeros(len(krange)))
for alg in algorithms])
k_results_build = {alg: np.zeros(len(krange))
for alg in algorithms}
k_results_query = {alg: np.zeros(len(krange))
for alg in algorithms}

X = get_data(N, DD, dataset)

Expand Down
2 changes: 1 addition & 1 deletion benchmarks/bench_plot_nmf.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class _PGNMF(NMF):
def __init__(self, n_components=None, solver='pg', init=None,
tol=1e-4, max_iter=200, random_state=None,
alpha=0., l1_ratio=0., nls_max_iter=10):
super(_PGNMF, self).__init__(
super().__init__(
Copy link
Member

Choose a reason for hiding this comment

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

All changes to super() are done in #12812 and should be reverted here..

Copy link
Member

@rth rth Jan 10, 2019

Choose a reason for hiding this comment

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

An easier solution might also be to wait until #12812 is merged and re-run pyupgrade then which will make this diff smaller.

n_components=n_components, init=init, solver=solver, tol=tol,
max_iter=max_iter, random_state=random_state, alpha=alpha,
l1_ratio=l1_ratio)
Expand Down
6 changes: 3 additions & 3 deletions benchmarks/bench_sample_without_replacement.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ def bench_sample(sampling, n_population, n_samples):

###########################################################################
# Remove unspecified algorithm
sampling_algorithm = dict((key, value)
for key, value in sampling_algorithm.items()
if key in selected_algorithm)
sampling_algorithm = {key: value
for key, value in sampling_algorithm.items()
if key in selected_algorithm}

###########################################################################
# Perform benchmark
Expand Down
2 changes: 1 addition & 1 deletion doc/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,6 @@ def setup(app):

# The following is used by sphinx.ext.linkcode to provide links to github
linkcode_resolve = make_linkcode_resolve('sklearn',
u'https://github.com/scikit-learn/'
'https://github.com/scikit-learn/'
'scikit-learn/blob/{revision}/'
'{package}/{path}#L{lineno}')
8 changes: 4 additions & 4 deletions doc/sphinxext/sphinx_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ def user_role(name, rawtext, text, lineno,
if config.issues_user_uri:
ref = config.issues_user_uri.format(user=target)
else:
ref = 'https://github.com/{0}'.format(target)
ref = 'https://github.com/{}'.format(target)
if has_explicit_title:
text = title
else:
text = '@{0}'.format(target)
text = '@{}'.format(target)

link = nodes.reference(text=text, refuri=ref, **options)
return [link], []
Expand All @@ -66,10 +66,10 @@ def _make_issue_node(issue_no, config, options=None):
if config.issues_uri:
ref = config.issues_uri.format(issue=issue_no)
elif config.issues_github_path:
ref = 'https://github.com/{0}/issues/{1}'.format(
ref = 'https://github.com/{}/issues/{}'.format(
config.issues_github_path, issue_no
)
issue_text = '#{0}'.format(issue_no)
issue_text = '#{}'.format(issue_no)
link = nodes.reference(text=issue_text, refuri=ref, **options)
else:
link = None
Expand Down
2 changes: 1 addition & 1 deletion doc/tutorial/machine_learning_map/parse_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class CaselessPreservingLiteral(CaselessLiteral):
instead of as defined.
"""
def __init__( self, matchString ):
super(CaselessPreservingLiteral,self).__init__( matchString.upper() )
super().__init__(matchString.upper())
self.name = "'%s'" % matchString
self.errmsg = "Expected " + self.name
self.myException.msg = self.errmsg
Expand Down
Loading