Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Cleanup: sorted, dict iteration, array.{ndim,size}, ... #7549
Conversation
tacaswell
added this to the
2.1 (next point release)
milestone
Dec 2, 2016
anntzer
changed the title from
Cleanup sorted to Cleanup: sorted, dict iteration
Dec 2, 2016
anntzer
changed the title from
Cleanup: sorted, dict iteration to Cleanup: sorted, dict iteration, array.{ndim,size}
Dec 2, 2016
anntzer
changed the title from
Cleanup: sorted, dict iteration, array.{ndim,size} to Cleanup: sorted, dict iteration, array.{ndim,size}, ...
Dec 2, 2016
|
Do Unicode entities work in Python 2? I'm not seeing it in the Unicode howto. |
|
Yes, see table at https://docs.python.org/2.7/reference/lexical_analysis.html#string-literals (or try it yourself :-)). |
codecov-io
commented
Dec 3, 2016
•
Current coverage is 61.90% (diff: 60.69%)@@ master #7549 diff @@
==========================================
Files 173 173
Lines 56103 55917 -186
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 34731 34614 -117
+ Misses 21372 21303 -69
Partials 0 0
|
| - line = fh.readline() | ||
| - if not line: | ||
| - break | ||
| + for line in fh: | ||
| line = line.rstrip() | ||
| if len(line) == 0: |
| @@ -135,9 +135,8 @@ def plot_figure(style_label=""): | ||
| # Setup a list of all available styles, in alphabetical order but | ||
| # the `default` and `classic` ones, which will be forced resp. in | ||
| # first and second position. | ||
| - style_list = list(plt.style.available) # *new* list: avoids side effects. | ||
| + style_list = sorted(plt.style.available) # *new* list: avoids side effects. |
| def values(self): | ||
| """ | ||
| Return values in order of sorted keys. | ||
| """ | ||
| - return [self[k] for k in self.keys()] | ||
| + return [self[k] for k in self] |
QuLogic
Dec 4, 2016
Member
Does dropping .keys() and iterating over self correctly preserve the sorted order that the keys method from this subclass produces?
anntzer
Dec 4, 2016
Contributor
No. But that also means that iteration on the dict itself (for k in rcparams) happens in a different order right now...
| @@ -91,7 +91,7 @@ def _sanity_check(fh): | ||
| # do something else with the file. | ||
| pos = fh.tell() | ||
| try: | ||
| - line = fh.readline() | ||
| + line = next(fh) |
anntzer
Dec 4, 2016
Contributor
The problem is that the outer loop may be using for line in fh first before passing the handle to the function at some point, and Py2 (not Py3) doesn't allow (raises an exception on) mixing for line in fh and fh.readline due to the fact that the former uses a readahead buffer (https://docs.python.org/2/library/stdtypes.html#file.next).
QuLogic
Dec 7, 2016
Member
But this is called before the iteration, and the next line is a seek, which should flush the readahead buffer anyway.
anntzer
Dec 7, 2016
Contributor
I think the gain in legibility with .readline() doesn't compensate the brittleness of only being able to pass in a file with an empty readahead buffer.
| @@ -232,20 +225,17 @@ def _parse_kern_pairs(fh): | ||
| """ | ||
| - line = fh.readline() | ||
| + line = next(fh) |
| - locs = [ti[1] for ti in tick_tups] | ||
| - locs.sort() | ||
| - locs = np.array(locs) | ||
| - if len(locs): |
QuLogic
Dec 4, 2016
Member
I understand this condition is probably not necessary, but I guess it could also be anded onto line 955?
anntzer
Dec 4, 2016
Contributor
In fact it's probably needed, otherwise we may end up trying to get the first or last item of an empty array...
| -IMAGE_FORMAT = ['eps', 'jpg', 'png', 'ps', 'svg'] + ['bmp'] # , 'raw', 'rgb'] | ||
| -IMAGE_FORMAT.sort() | ||
| -IMAGE_FORMAT_DEFAULT = 'png' | ||
| +IMAGE_FORMAT = sorted(['eps', 'jpg', 'png', 'ps', 'svg'] + ['bmp']) # , 'raw', 'rgb'] |
anntzer
Dec 4, 2016
Contributor
Sure, but I'll keep the sorted which makes the intent clear and should be cheap.
| - | ||
| - if len(seq1) != len(seq2): return False | ||
| - return np.alltrue(np.equal(seq1, seq2)) | ||
| +seq_allequal = np.array_equal |
anntzer
Dec 4, 2016
Contributor
Strictly speaking, it was; in practice I'm 100% sure we can just get rid of it...
| @@ -1652,55 +1638,48 @@ def pstoeps(tmpfile, bbox=None, rotated=False): | ||
| bbox_info, rotate = None, None | ||
| epsfile = tmpfile + '.eps' | ||
| - with io.open(epsfile, 'wb') as epsh: | ||
| + with io.open(epsfile, 'wb') as epsh, io.open(tmpfile, 'rb') as tmph: |
anntzer
Dec 7, 2016
Contributor
I guess...
On the other hand there seems to be an awful lot use of temporary files in backend_ps, most of which could probably just get rewritten using pipes to communicate via the subprocesses' standard streams.
Another great cleanup project :-)
| @@ -331,8 +328,7 @@ def _get_grid_bbox(self, renderer): | ||
| Only include those in the range (0,0) to (maxRow, maxCol)""" | ||
| boxes = [self._cells[pos].get_window_extent(renderer) |
QuLogic
Dec 4, 2016
Member
Since the value is actually used here, it might make sense to use items() below.
| @@ -186,8 +186,8 @@ def set_params(self, **kwargs): | ||
| self.den = int(kwargs.pop("nbins")) | ||
| if kwargs: | ||
| - raise ValueError("Following keys are not processed: %s" % \ | ||
| - ", ".join([str(k) for k in kwargs.keys()])) | ||
| + raise ValueError("Following keys are not processed: %s" |
| - p0, p1 = edges[edgei] | ||
| + p0, p1 = min(self.tunit_edges(), | ||
| + key=lambda edge: proj3d.line2d_seg_dist( | ||
| + edge[0], edge[1], (xd, yd))) |
| - tis = (vecw[0] >= 0) * (vecw[0] <= 1) * (vecw[1] >= 0) * (vecw[1] <= 1) | ||
| - if np.sometrue(tis): | ||
| - tis = vecw[1] < 1 | ||
| + tis = (vecw[0] >= 0) & (vecw[0] <= 1) & (vecw[1] >= 0) & (vecw[1] <= 1) |
QuLogic
Dec 4, 2016
Member
Since you're editing the line already, can you reorder it so that it looks like an implied and?
| @@ -28,7 +28,7 @@ def terminate(self): | ||
| def poll_draw(self): | ||
| def call_back(): | ||
| - while 1: | ||
| + while True: |
Kojoley
Dec 4, 2016
Member
Cannot this be while self.pipe.poll(): (and remove if not self.pipe.poll(): break below)?
| - while 1: | ||
| - | ||
| - if len(remaining) == 0: | ||
| + while True: |
| + if not self.figure.canvas.is_saving(): | ||
| + artists = [a for a in artists | ||
| + if not a.get_animated() or a in self.images] | ||
| + artists = sorted(artists, key=lambda artist: artist.get_zorder()) |
| - spec = datad[cmapname] | ||
| - spec_reversed = _reverse_cmap_spec(spec) | ||
| - datad[cmapname + '_r'] = spec_reversed | ||
| + for cmapname, spec in list(six.iteritems(datad)): |
anntzer
Dec 4, 2016
•
Contributor
No because we're modifying the dict at the same time; sidestepped the issue using update.
| @@ -350,8 +346,7 @@ def contains(self, mouseevent): | ||
| renderer = self.figure._cachedRenderer | ||
| if renderer is not None: | ||
| boxes = [self._cells[pos].get_window_extent(renderer) | ||
| - for pos in six.iterkeys(self._cells) | ||
| - if pos[0] >= 0 and pos[1] >= 0] | ||
| + for pos in self._cells if pos[0] >= 0 and pos[1] >= 0] |
QuLogic
Dec 5, 2016
Member
I knew I should have commented the first time, but here's another one that could use items.
|
Can you rebase this PR? |
NelleV
changed the title from
Cleanup: sorted, dict iteration, array.{ndim,size}, ... to [MRG+1] Cleanup: sorted, dict iteration, array.{ndim,size}, ...
Dec 19, 2016
| - style_list.sort() | ||
| - style_list.insert(0, u'default') | ||
| - style_list.insert(1, u'classic') | ||
| + style_list = ['default', 'classic'] + sorted( |
| - line = fh.readline() | ||
| - if not line: | ||
| - break | ||
| + for line in fh: |
|
After Christmas break, probably. |
anntzer
added some commits
Dec 2, 2016
|
Actually it wasn't that bad. |
anntzer commentedDec 2, 2016
•
edited
sortedwhereever it improves readbility overlist.sort.iterkeys.N{...}unicode entities.