MAINT: Remove python <2.7,<3.3 string/unicode workarounds

[eric-wieser]

This transforms strings to use the u and b prefixes for unicode and bytes:

• asbytes('hello') → b'hello'
• asbytes_nested(['a', 'b']) → [b'a', b'b']
• asunicode('hello') → u'hello'
• unicode('hello') → u'hello'
• sixu('hello') → u'hello'

This is fine because b is supported on 2.7 and 3.x, and u is supported on 2.x and 3.3+. Our minimum versions are now 2.7 and 3.4?

What we can't do is transform asbytes("tests %d" % num), because %-formatting
fails on bytes in python 3.x < 3.5.

As a result, asunicode and sixu are now not used anywhere.

[juliantaylor]

thanks, nice cleanup
though seeing this used so often just reminds me of the poor state our io functions are in...

[eric-wieser]

though seeing this used so often just reminds me of the poor state our io functions are in

Tell me about it. Perhaps now that we don't use these conversion/hack functions much in testing, they can give a deprecation warning on python 3?

[eric-wieser]

Also, is removing functions from numpy.compat ok, or is that public api?

[juliantaylor]

technically public api, we could slap deprecation warnings on them but I don't think its worth the effort.

[eric-wieser]

Hmmm... This had a big performance hit on bench_core.CountNonzero.time_count_nonzero(1, 1000000, <type 'str'>), which is a little surprising.

Especially since none of the things this touches should have any effect on the behaviour of count_nonzero...

[eric-wieser]

Ah, that regression is clearly from 9791f20, and is due to a deliberate benchmark change.

@pv: Am I reading ASV incorrectly, or does it look like there's a bug in attaching commit ids to benchmarks?

[pv]

ASV does not try to track if the *benchmark* code itself is changed. If benchmarks are changed, old results need to be invalidated manually. The results repository in question is here: https://github.com/pv/numpy-bench

[eric-wieser]

ASV does not try to track if the benchmark code itself is changed

Sure, that explains why the regression is there. But why does it attach the regression to this commit, and not to your commit? It has benchmark data for both, and mine came before yours!

On a related note, does this mean this was a bad merge, and the benchmark should have been renamed to avoid creating these false regressions?

[pv]

The benchmark setup runs it once per day, using the latest benchmark suite in the master branch. . The benchmark suite and the numpy code should be thought as independent entities --- that they are stored in the same repository is just a matter of convenience for pull requests. . In particular, the benchmark code used to benchmark each commit is independent of the commit benchmarked (otherwise, it would not be possible to fix mistakes in benchmarks afterward). Hence, the benchmark suite and code commit ids are not comparable in general. . In principle changes in benchmarks could be tracked automatically by looking for source code changes. However, this can result to false positives, so currently you have to do the invalidation manually.

[eric-wieser]

Gotcha, that clarifies things

Does this mean that if a pull request introduces a benchmark and a fix, that the benchmark will be tested on the previous commit as well?

Or did that only happen in this case because the two commits were merged on the same day?

[pv]

Since it's run only once per day, benchmark additions or modifications apply also to other commits made during the same day --- including also any such commits *preceding* the change in the benchmarks. . If you are making changes to benchmark suite that changes results, either the benchmark name should be changed or the old results be invalidate. . Alternatively, you can send a PR to asv that implements automatic invalidation of results :) I don't see how to do this reliably however, so probably it should be optional.

[juliantaylor]

automatic invalidation worked pretty well in the vbench suite. It took the checksum of the benchmark and invalided the results when it changed.

[pv]

Probably the 90+% solution indeed is just to take a hash of the inspect.getsourcelines of the benchmark and setup methods.

[pv]

... and the hash (or the source lines themselves) can be stored in the result json files, and comparisons be done at publish stage. Sounds simple.

[eric-wieser]

Perhaps a simple "version" field in benchmarks would do the job too