Significant performance improvement in bag.core._chunk_read_file

This PR changes the way a single large text file is processed in parallel using from_filenames and the chunkbytes argument. Before, in utils.textblock utils.next_linesep determined the exact start and end bytes of each block by actually streaming over the corresponding region in the file and finally the exact region was loaded into memory. This is very inefficient because the region is read twice and loaded into memory while the remaining graph may only require streaming over the bag elements. I noticed the performance problem when processing both a large and even very moderately sized (a couple of MBs) gzipped text file. This PR makes the chunked processing significantly faster.
I have only made changes to the bag module but saw that utils.textblock is also used in the dataframe module. Perhaps my bag.core._textblock function can be moved to utils and then dataframe may be adapted to benefit from the speed-up, too.
I welcome any comments and suggestions.

Could someone double-check my handling of different encodings? I tried to get it right and tests are passing locally, but I'm not 100% sure it's handled correctly when searching for the line separator and later during the actual decoding of a chunk.

Some tests are not passing here, but I didn't touch any of the Pandas related parts. Any idea?

Hi @sisp, some thoughts

This is great!  Thanks both for identifying and for resolving this issue.
Can you share a benchmark or dataset on which this change yields a clear performance gain?
Indeed, it looks like next_linesep is doing several repeated reads over the same line (as many reads as characters.)  Presumably this performs poorly for files where you're trying to find the endline of a very long line.  It looks like you're depending on readline to manage this instead.
Ideally we could roll this into the base _textblock function so that, as you suggest, it could benefit dask.dataframe as well.

@cowlicks, can you comment on the approach here?  You're probably most familiar with endline and character encodings.
I'll take a look at the Pandas related issues tomorrow morning.  It's possible that an upstream dependency has changed.  I suggest ignoring these issues for now.

Thanks for your comments @mrocklin. I'll benchmark the performance gain for a couple of cases and post them here.
I've added a few line comments so that I don't forget about them later today when I have time to work on this.
When running the modified code yesterday, I noticed another issue that I don't know how to solve though. Files are chunked using the file_size utility function, but the implementation for gzipped files only works for files up to 4GB. I only noticed this because my output files were missing many data points. I googled a little to find a solution, but it seems there's no cheap way to determine the uncompressed file size of a large gzipped file. I'm not even sure how to print a warning because I don't know apriori (unless. I guess, the gzip file itself is larger than 4GB) wether the uncompressed file size is too large for a 32 bit int (as used in file_size).

... the implementation for gzipped files only works for files up to 4GB.

This is a known issue (https://github.com/blaze/dask/issues/576). If you can think of a good solution to this, that would be much appreciated. But I don't think there is one.

I also don't know of a good solution for the gzip issue.  The best I can think of is better warnings and better documentation.

Perhaps we should say something more explicit about this issue in docstrings like from_filenames
Perhaps we should warn when using large-ish gzip files (like larger than 500 MB)

But as you suggest, there is no way to really properly defend against this fail case.

Hello @sisp, I basically re-implemented the readline method because it does not allow you to specify a line separator. However if you open a file with the io.open (instead of the stdlib open) you can tell the file what its linesep is, so the readline method will behave properly. The caveat is that gzip.open does not take a linesep argument.
We could make a readline function that reads in larger blocks (instead of one byte at a time) and checks for lineseps. This would do fewer larger reads instead of many smaller reads.

The pandas issues have been resolved.  If you rebase/merge on master tests should pass.

@mrocklin I am not aware of any cheap way to determine the uncompressed file size of a gzipped file either.
@cowlicks How about using io.TextIOWrapper (https://docs.python.org/2/library/io.html#io.TextIOWrapper)? This would work with any compression method. Or do you need arbitrary line separators? io.TextIOWrapper only accepts '', '\n', '\r', and '\r\n' according to the website.

I think that \n, \r, and \r\n should be sufficient.  I've never seen a format that went beyond these.
It looks like you're running into a Python 3 compatibility issue in the tests.
I'm a bit hesitant to have two textblock functions within dask for maintenance reasons.  @sisp what are your thoughts on generalizing your approach so that it can be more broadly useful to other collections?

I've been trying to use io.TextIOWrapper, but the text encoding is giving me a bit of a headache. I'm getting errors such as UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-2: ordinal not in range(128) or  assert ['Hello, worl...llo, world!\n'] == ['Hello, world...world!\n', ...] while running the tests.
@cowlicks @mrocklin About providing line separators, I've been wondering if it's really important to be able to do this, why is the non-chunked reader fine without this feature (https://github.com/blaze/dask/blob/master/dask/bag/core.py#L869-L870)?
@mrocklin I'd be happy to move bag.core._textblock to utils once the implementation is stable. I agree there shouldn't be two very similar implementations.

@sisp if you push your most recent branch, and point out the tests your talking about I can take a look.
I think the stdlib open defaults to the OS line separator. A problem could arise if you try to load data not created on your platform. Linux uses \n, windows uses \n\r.

I currently feel like the custom readline implementation with arbitrary line separators is better than acrobatics with io.TextIOWrapper. It's not as straightforward as I had hoped it to be.
The last commit is breaking Python3 tests.
E                   dask.async.AttributeError: 'bytes' object has no attribute 'encode'
E
E                   Traceback
E                   ---------
E                     File "/home/travis/build/blaze/dask/dask/async.py", line 264, in execute_task
E                       result = _execute_task(task, data)
E                     File "/home/travis/build/blaze/dask/dask/async.py", line 246, in _execute_task
E                       return func(*args2)
E                     File "/home/travis/build/blaze/dask/dask/bag/core.py", line 826, in decode_sequence
E                       for item in seq:
E                     File "/home/travis/build/blaze/dask/dask/bag/core.py", line 938, in _textblock
E                       pos = next(lines)[1]
E                     File "/home/travis/build/blaze/dask/dask/bag/core.py", line 880, in _readlines
E                       bin_linesep = get_bin_linesep(encoding, linesep)
E                     File "/home/travis/build/blaze/dask/dask/utils.py", line 166, in get_bin_linesep
E                       return linesep.encode(encoding)

However, I don't see what's going wrong there. When I run os.linesep.encode('utf-8') with Python 3.4.3, I don't get any error that bytes has no attribute encode. What is going wrong here?
Concerning a performance benchmark between the original implementation and the last commit of this PR: I ran the original implementation on a ~12MB gzipped file with chunkbytes=1e7 for more than an hour (a simple identity mapping) and dask hadn't even started writing output files, so I terminated the process eventually. In contrast, this PR's implementation finishes in ~5 seconds on my laptop. I believe https://github.com/blaze/dask/blob/master/dask/utils.py#L174-L179 is the severe bottleneck. It essentially starts reading from position seek an increasing number of data_len bytes as long as the line separator is not found (so the same bytes are read over and over again) and the line separator is searched for using bin_linesep in data which gets increasingly expensive for larger data and is unnecessary because only the last len(bin_linesep) characters of data would need to be checked.
Once we figure out why Python 3 tests are failing and how to fix them, I would suggest to move the code to utils and adapt the code in dataframe accordingly.
Further, from_filenames should have a linesep argument, too. Otherwise, it wouldn't be possible to tell it which line separator to use. Finally, the non-chunked implementation should be modified to support arbitrary line separators.

Taking a look at the linesep issue now

So it looks like os.linesep is a string by default but that in get_bin_linesep we encode this string into bytes in order to handle Byte Order Marks.
def get_bin_linesep(encoding, linesep):
    """
    Simply doing `linesep.encode(encoding)` does not always give you
    *just* the linesep bytes, for some encodings this prefix's the
    linesep bytes with the BOM. This function ensures we just get the
    linesep bytes.
    """
    if encoding == 'utf-16':
        return linesep.encode('utf-16')[2:]  # [2:] strips bom
    else:
        return linesep.encode(encoding)

We should probably never call encode on the result of this function (often called bin_linesep).  It looks like the _textblock function gets the string linesep, either from a keyword argument or from os.linesep and then converts this to bin_linesep using the get_bin_linesep function above.  What's odd is that _textblock then passes this bin_linesep when calling _readlines, which again calls get_bin_linesep on the result.  Or, more concisely

_textblock converts linesep -> bin_linesep
_textblock passes bin_linesep to _readlines
_readlines again tries to convert bin_linesep, raising the error above.

Thanks for looking into this issue. I've modified the code to avoid applying get_bin_linesep repeatedly. I hope the Python 3 tests are passing now (couldn't check it locally because I don't have a full Python 3 environment set up -- if they don't pass, I may have to set one up ...).

Tests are passing now. Could you review the current state in general, i.e. see whether the handling of encoding (byte-level line separator, determining the encoding etc.) and the general approach look good? Once you're good with it, I would start moving textblock and readlines to utils and adapt dataframes code accordingly.

The general approach here seems sane to me.  I think that it's unfortunate that we've had to reimplement the readline logic that must be present (and faster) in various standard libraries, but this is a tricky subject and the current state is sufficiently broken performance-wise that this is a meaningful and pragmatic improvement.  I'm curious, what issues did you run into with io.TextIOWrapper? (I was unfamiliar with the existence of this object until now)
I'm somewhat ignorant of the details of character encodings.  I hope that @cowlicks can offer some more detailed advice here.  @cowlicks I found reviewing the PR without the pep8 changes to be handy.  This view is available here: https://github.com/blaze/dask/compare/92c8e67be0336a14dd1151271230ab36729ab31a...sisp:chunked_file_reader?expand=1

Perhaps I should have fixed pep8 issues in a different PR.
Concerning io.TextIOWrapper, I think I've sorted out the encoding issues. This is the code that passes the tests locally:
opens = {'gz': gzip.GzipFile, 'bz2': bz2.BZ2File}

def _textblock(filename, start, end, compression, encoding=None, linesep=None):
    with io.open(filename, 'rb') as f:
        try:
            fc = opens[compression](fileobj=f)
        except KeyError:
            fc = f
        try:
            with io.BufferedReader(fc) as fcb:
                # Get text file encoding.
                if encoding is None:
                    encoding = 'utf-8'
                # Get line separator.
                if linesep is None:
                    linesep = os.linesep
                # Get byte representation of the line separator.
                bin_linesep = get_bin_linesep(encoding, linesep)
                bin_linesep_len = len(bin_linesep)

                # Determine which of the following cases applies:
                # (1) skip=False: `start` corresponds to the beginning of a
                #     line. We can start reading lines of this text block.
                # (2) skip=True: `start` corresponds to a position in the
                #     middle of a line. We need to skip characters until we
                #     reach the beginning of the next line.
                skip = False
                if start >= bin_linesep_len:
                    fcb.seek(start - bin_linesep_len)
                    if fcb.read(bin_linesep_len) != bin_linesep:
                        skip = True
                else:
                    fcb.seek(start)

                # Skip characters to reach the beginning of the next line
                # if necessary.
                if skip:
                    start += len(next(fcb))

                with io.TextIOWrapper(fcb,
                                      encoding=encoding,
                                      newline=bin_linesep) as fcbw:
                    # Retrieve and yield lines until the file position exceeds
                    # the `end` position.
                    try:
                        while start < end:
                            line = next(fcbw)
                            yield line
                            # We need to encode the line again to get the byte
                            # length in order to correctly update the byte
                            # count.
                            start += len(line.encode(encoding))
                    except StopIteration:
                        pass
        finally:
            fc.close()

I didn't want to update the PR yet because there are a few things to discuss first. First, we need to make sure we're at the beginning of a line. We use the chunk offsets which are only good at the byte-level as decoding multibyte encodings fails when the offset points somewhere in the middle of a multibyte character. Therefore, finding the correct offset must happen at the byte-level, i.e. using io.BufferedReader (not io.TextIOWrapper). Once we have the correct offset, we can use io.TextIOWrapper to read lines using our custom line separator. An issue with this approach is that io.TextIOWrapper which always decodes bytes according to the encoding type. In order to keep track of the actual file pointer position, we need to count the number of bytes we have decoded (tell() is not available anymore) so we need to encode the decoded bytes again just to get the correct number of bytes. This is wasteful but perhaps not terrible.

First, let me just say how great it is to have other people thinking about this.  I really appreciate it.
How expensive is encoding/decoding in the common case?
In [1]: s = ''.join(map(str, range(10))) * 100000  # 1MB of utf-8 / ASCII

In [2]: %time bytes = s.encode()
CPU times: user 854 µs, sys: 91 µs, total: 945 µs
Wall time: 952 µs

In [3]: %time text = bytes.decode()
CPU times: user 803 µs, sys: 93 µs, total: 896 µs
Wall time: 905 µs

It looks like in the common case of utf-8 that is also compatible ASCII (requiring no decoding) we're operating at about 1GB/s, or a small factor slower than memory copy speeds and a bit faster than standard disk reading speeds.
However, this is only for large chunks (1MB) and presumably if we're slowly walking through a file this would need to happen repeatedly, which presumably hurts performance significantly.
In [15]: s = ''.join(map(str, range(10))) * 1000  # 10kB of utf-8 / ASCII

In [16]: %time bytes = s.encode()
CPU times: user 0 ns, sys: 0 ns, total: 0 ns
Wall time: 18.8 µs

In [17]: %time text = bytes.decode()
CPU times: user 0 ns, sys: 0 ns, total: 0 ns
Wall time: 33.9 µs

In [18]: 10e3 / 30e-6 / 1e6  # MB/s
Out[18]:<PHONE_NUMBER>333333

Turns out its still not bad.

Your performance benchmark looks promising. I've just discovered a bug in my code, though.
if skip:
    start += len(next(fcb))

Calling next(fcb) is actually wrong because fcb is the io.BufferedReader object which again does not accept custom line separators. So we can't control where next(fcb) ends up in the file. But calling next(fcbw) does not work either because we don't know if the current file offset is somewhere in the middle of a multibyte character. We would need a raw byte reader that accepts a custom line separator. Uh, this is more involved than I had thought...

I think I'm getting closer to figuring out how to do this.
def _textblock(filename, start, end, compression, encoding, linesep):
    f = opens.get(compression, io.open)(filename, 'rb')
    try:
        with io.BufferedReader(f) as fb:
            # Get byte representation of the line separator.
            linesep = get_bin_linesep(encoding, linesep)
            linesep_len = len(linesep)

            # If `start` does not correspond to the beginning of the file, then
            # we need to set the file pointer to `start` and keep reading
            # chunks until we find a line separator. We then set the file
            # pointer to the position after the line separator.
            # If `start` corresponds to the first character of a line, the line
            # is not part of this textblock. This is okay since
            # io.TextIOWrapper below keeps reading lines until `start` exceeds
            # `end`, i.e also the line where `start == end` is part of this
            # textblock. Thus, the line is definitely only part of one
            # textblock.
            if start > 0:
                fb.seek(start)
                while True:
                    buf = f.read(4096)
                    try:
                        start += buf.index(linesep)
                        start += linesep_len
                    except ValueError:
                        start += len(buf)
                    else:
                        f.seek(start)
                        break

            with io.TextIOWrapper(fb, encoding, newline=linesep) as fbw:
                try:
                    # Retrieve and yield lines until the file position
                    # *exceeds* `end`. See the comment above for why
                    # `start <= end` is correct and not `start < end`.
                    while start <= end:
                        line = next(fbw)
                        yield line
                        # We need to encode the line again to get the byte
                        # length in order to correctly update the byte offset.
                        start += len(line.encode(encoding))
                except StopIteration:
                    pass
    finally:
        f.close()

Tests are passing locally (I just adapted the expected dask graph accordingly) and my own use case is  working correctly with this implementation. I saw your comment about handling the linesep problem. I wonder what happens with 2 * bin_linesep_len when we have A\r\n\r\nB\r\n. The way I believe to have solved it works as follows: Unless start == 0, we advance the file pointer to the beginning of the next line (even if start happens to point to the beginning of a line). In order to ensure that we don't miss this line (where start points to the beginning of the line), we keep reading lines while start <= end (I think it is important to use <= and not < here). If I'm not mistaken, this should work fine. What do you think?
There's one other situation that I'm not yet clear about. What happens if chunksize is much smaller than some line length? Wouldn't multiple text blocks contain the same line if start of two text blocks points to some position in the middle of the same line?

I'm not sure how to handle the case where chunksize is smaller than line length.  Honestly I'd be fine raising an informative error in that case asking the user to increase the chunksize.

There are two problems now: (1) io.TextIOWrapper expects the newline argument not to be a byte string in all version of python but 2.7. I've fixed this locally; (2) io.BufferedReader(gzip.open(...)) is not supported in python 2.6 throwing AttributeError: GzipFile instance has no attribute '_checkReadable'. I haven't yet figured out how to solve this, but I'm working on it.

It seems in Python 2.7+, gzip.GzipFile inherits from io.BufferedIOBase which again inherits from io.IOBase implementing _checkReadable which simply calls the method readable which is not implemented in gzip.GzipFile in Python 2.6. I believe we're facing an interface incompatibility here. I could extend gzip.GzipFile in the compatibility module, mix in io._BufferedIOMixin, and implement the methods that gzip.GzipFile is missing in Python 2.6. Does this sound sensible?

Sorry, probably a dumb question, but what stops us from

Being very clever to determine clean chunk boundaries
Then reading the entire large chunk of bytes at once
Then sending this large chunk of clean uncompressed bytes to something like TextIOWrapper.

This would eliminate the need for our custom encoding and endline-finding logic on each line (though we would still need it on the chunk boundaries) and it would probably also remove the need to have things like gzip interact strongly with other systems (once we read bytes out of the gzip file we can forget compression.)

Do you mean we would read the entire chunk into memory? Probably into something like StringIO? If, so, that may increase memory usage (significantly, if the chunk size is large).

I think that it's reasonable to assume that chunksize fits conveniently into memory.  You're right though that a fully lazy solution would be better.  This is probably a trade-off in code complexity/maintenance vs performance.
I'm not sure what the relative costs/benefits are here; you probably have a better handle on this.

One argument against this approach -- besides the question whether in-memory chunks are desirable or not -- is that also the non-chunked case requires custom line separators using, e.g., io.TextIOWrapper, so we have the same problem there except that we cannot load the entire file into memory.

Oh yes, that's a good point

Sorry for having been quite for a while.
I've added compatibility wrappers for GzipFile and BZ2File locally and they appear to be compatible with TextIOWrapper in Python 2.6. I haven't had time to create a proper environment to test the whole package with Python 2.6 though.
I've noticed that Blaze has dropped Python 2.6 support. Any chance Dask is going to follow? The current issue of this PR would be resolved if Python 2.6 compatibility was not enforced anymore. Just a thought ...

@sisp No, we have users and downstream projects which depend on Python 2.6
It would be nice to have dictionary comprehensions though.

Glad to see this rekindled!  I'll be able to take a closer look at this tomorrow morning.  I'm running into some time conflicts due to the holidays.
I would love to drop 2.6.  I'd like to keep dask.core an infrastructural project, which probably means supporting most python versions.  It's possible that we could drop support within certain collections though.  I wouldn't want to do this just yet though.

@sisp how can I help to test out your GzipFile and BZ2File compatibility wrappers in py26?
FWIW, I use conda to set up my python version environments (though I'm probably a bit biased here)
conda create -n py26 python=2.6 dask pytest ipython

@mrocklin Thanks for the pointer to conda's easy way of creating environments with different Python versions. I've finally tested the code locally and with travis, both are passing now. I've moved textblock to dask.utils and adapted the dask.dataframe code to use the new textblock as well. Handling utf-16 encoding was a bit cumbersome and I found a corner case in textblock that needed a fix, but I believe it's working now. Let me know what you think of the current solution.

Was doing a bit of before/after performance comparisons on 800MB of gzip compressed NYCTaxi data
Bag
In [1]: import dask.bag as db
In [2]: b = db.from_filenames('trip_data_1_*.csv.gz')
In [3]: import dask
In [4]: %prun b.count().compute(get=dask.get)

Before
         75970747 function calls (75970498 primitive calls) in 81.930 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
 14776631   12.860    0.000   79.848    0.000 core.py:823(decode_sequence)
 14776616    6.761    0.000    6.761    0.000 {method 'decode' of 'bytes' objects}
 15712988    6.392    0.000    6.392    0.000 {method 'find' of 'bytes' objects}
   936327    3.363    0.000    6.370    0.000 gzip.py:469(_add_read_data)
   936357    3.015    0.000   29.457    0.000 gzip.py:349(read)
   936359    2.802    0.000    2.802    0.000 {built-in method crc32}
   936344    2.095    0.000   25.163    0.000 gzip.py:425(_read)
       15    2.071    0.138   81.919    5.461 {built-in method count}
   936674    1.934    0.000    1.934    0.000 {method 'read' of '_io.BufferedReader' objects}

After
         21114659 function calls (21114260 primitive calls) in 32.707 seconds

   Ordered by: internal time
objects}
       15    7.706    0.514   32.692    2.179 {built-in method count}
   535871    2.543    0.000    2.543    0.000 {built-in method crc32}
   535826    1.931    0.000    4.580    0.000 gzip.py:469(_add_read_data)
 15077061    1.645    0.000    1.645    0.000 gzip.py:498(closed)
   300280    1.462    0.000   22.422    0.000 gzip.py:349(read)
   536186    1.218    0.000    1.218    0.000 {method 'read' of '_io.BufferedReader' objects}
   535856    1.126    0.000   20.467    0.000 gzip.py:425(_read)
   300265    0.571    0.000    0.571    0.000 {built-in method utf_8_decode}
   300265    0.392    0.000    0.963    0.000 codecs.py:316(decode)
   535826    0.313    0.000    0.313    0.000 {built-in method min}
   536201    0.297    0.000    1.516    0.000 gzip.py:79(read)
   300310    0.137    0.000    0.180    0.000 gzip.py:237(_check_closed)
  1071825    0.106    0.000    0.106    0.000 {built-in method len}

DataFrame
In [5]: import dask.dataframe as dd
In [6]: df = dd.read_csv('trip_data_1_*.csv.gz', skipinitialspace=True, compression='gzip')
In [7]: %prun df.trip_distance.compute(get=dask.get)

Before
cancelled because it was taking so long
After
         142944297 function calls (142938964 primitive calls) in 246.672 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
  4272762   67.118    0.000   67.118    0.000 {method 'decompress' of 'zlib.Decompress' objects}
       74   38.323    0.518   38.323    0.518 {method 'read' of 'pandas.parser.TextReader' objects}
  9868809   22.408    0.000  152.454    0.000 gzip.py:349(read)
  4275600   16.747    0.000   16.747    0.000 {method 'read' of '_io.BufferedReader' objects}
  4272762   13.629    0.000   27.769    0.000 gzip.py:469(_add_read_data)
 14776634   13.547    0.000   43.971    0.000 {built-in method next}
  4272925   13.319    0.000   13.319    0.000 {built-in method crc32}
 14776690   12.765    0.000  194.250    0.000 utils.py:189(textblock)
  4272792    8.530    0.000  122.504    0.000 gzip.py:425(_read)
       74    5.614    0.076  244.100    3.299 io.py:33(_read_csv)
      192    5.359    0.028  130.410    0.679 gzip.py:553(seek)
 14776691    4.281    0.000    4.281    0.000 {method 'encode' of 'str' objects}
  9868957    3.749    0.000    5.347    0.000 gzip.py:237(_check_closed)
 24646313    3.606    0.000    3.606    0.000 gzip.py:498(closed)
     1738    3.292    0.002  197.542    0.114 {method 'join' of 'str' objects}
23339863/23337271    2.354    0.000    2.355    0.000 {built-in method len}
  4275615    2.335    0.000   19.083    0.000 gzip.py:79(read)
  4273502    2.194    0.000    2.194    0.000 {built-in method min}
       77    1.290    0.017    1.290    0.017 {built-in method open}
      222    0.910    0.004    1.179    0.005 internals.py:4087(_stack_arrays)

Conclusions
Clearly there are some pretty serious improvements here.  There might be room for more (in a future PR).  It's odd that the dataframe case is so much slower.  This may be because it is seeking into a gzip file many times rather than just doing a single flow through the dataset (how seekable is gzip?)
There are a lot of function calls going on here.  I wonder if the current approach has non-trivial overhead that could be reduced by larger bulk operations.

Everything here looks pretty solid to me and it's a clear performance improvement.  I'm +1

Merging in 24 hours if no comments

I've added a few comments and would like to address them before you merge this PR. I would suggest not to merge yet.

Generally, tests are passing, but Python 2.7 tests are timing out when run with Travis.
dask/dataframe/tests/test_io.py::test_to_castra
No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.
The build has been terminated

Tests are passing locally with all Python versions, but I sometimes get a segmentation fault. Have you had this problem before?

I've rerun the tests.  The recent pandas release triggered some intermittent failures with the dask.dataframe tests.  These are unrelated to your work here.

@sisp are there any other obvious tasks to do here?  Are common failure cases sufficiently well tested that we can trust other people to make changes to this code without violating assumptions?

I've made some final modifications to the compatibility implementations of GzipFile and BZ2File which now avoid multiple inheritance and make utils.open fully compatible with with-statement syntax. I've also added a few more tests that check the correctness of utils.textblock when used with compression. The previous commit added code to properly deal with multi-byte line separators including tests. I believe everything is tested sufficiently, so if you don't have any additional comments, feel free to merge the PR.

@sisp thank you so much for your continued work on this.  I'll give this another look in about an hour and then probably merge.  I'll likely issue a release shortly afterwards.  This PR is very valuable.

I'm glad to help. Thanks for all the discussions that helped me a lot in making my code better!

This looks good to me.  I'm going to go ahead and merge.  I had some thoughts while looking through this

It would be good to have a full parametrized test around textblock for encodings, texts, compression systems, etc..
Blocked GZip reading is slow, and our existing algorithms within dask.dataframe.read_csv are inefficient.

Thanks for all of your work here @sisp


feat: finalize presentation flow.

Signed-off-by: Volodymyr Kubiv<EMAIL_ADDRESS>

Codecov Report
Base: 90.83% // Head: 90.44% // Decreases project coverage by -0.38% :warning:

Coverage data is based on head (3c092eb) compared to base (9c6163a).
Patch coverage: 73.91% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
- Coverage   90.83%   90.44%   -0.39%
==========================================
  Files          36       36
  Lines        1167     1193      +26
==========================================
+ Hits         1060     1079      +19
- Misses         66       71       +5
- Partials       41       43       +2

Impacted Files
Coverage Δ

cmd/wallet-sdk-gomobile/did/keywrapper.go
71.42% <ø> (+15.87%)
:arrow_up:

cmd/wallet-sdk-gomobile/localkms/localkms.go
78.94% <ø> (-2.01%)
:arrow_down:

pkg/localkms/localkms.go
80.00% <ø> (-1.82%)
:arrow_down:

pkg/localkms/ariescryptowrapper.go
88.23% <33.33%> (-11.77%)
:arrow_down:

cmd/wallet-sdk-gomobile/api/diddocresolution.go
76.19% <37.50%> (-17.93%)
:arrow_down:

cmd/wallet-sdk-gomobile/credential/inquirer.go
90.90% <75.00%> (-3.21%)
:arrow_down:

cmd/wallet-sdk-gomobile/openid4vp/openid4vp.go
93.93% <100.00%> (+0.18%)
:arrow_up:

pkg/common/jwt_signer.go
90.47% <100.00%> (+5.86%)
:arrow_up:

pkg/openid4vp/openid4vp.go
88.88% <100.00%> (+0.10%)
:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


Alphabetically sorted guidebook entries

About the PR

Added alphabetical guidebook sorting logic and removed unnecessary tree expansion call.
Why / Balance

https://github.com/space-wizards/space-station-14/issues/24770
Technical details

Sorts the children of a given guidebook entry first by their priority and then by their localized name, inside the prior GetSortedRootEntries loop. Also removed what appeared to be an unnecessary Tree.SetAllExpanded(true) call in RepopulateTree. Added readability to prior query variables.
Media

[X] I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Renamed GetSortedRootEntries to GetSortedEntries, currently only one reference within GuidebookWindow.xaml.cs
Changelog

:cl:

tweak: Guidebook entries are now alphabetically sorted

Starting test run
[Passed] IgnoredComponentsExistInTheCorrectPlaces
==== Summary ====
Passed!  - Failed:    0, Passed:    1, Skipped:    0, Total:    1, Duration: 11s

Odd, failed test works locally.

makes the two most important categories (chemicals and bartender) middle/top of the list so good change


Constrain prefetch semantics and functionality to align with HL7 Clinical Reasoning

Adds restrictions to the allowed prefetch queries to reduce EHR implementer burden and align with the DataRequirement of FHIR Clinical Reasoning.
This pull request addresses issue #47.

@brynrhodes - Can you clarify the restrictions further? For example:

What does "instance level read interactions" mean? I'm guessing this read in FHIR? If my understand of that is correct, what does the further qualification of "for the Patient resource only" mean?

Does "type level search interactions" only mean search and not create or history? Why is GET called out? The EHR can satisfy the prefetch data however it sees fit and I'm not sure what GET vs POST would mean

Are these similar restrictions defined similar to how you have documented then here somewhere in Clinical Reasoning? If so, where?
My questions here are just an attempt to ensure that I and everyone else understands very clearly what we're proposing is & isn't allowed in prefetch queries. 😄

I don't think these restrictions should be global. There might not even be a patient in context. More generally, what is most effective varies widely by context. There might not be the possibility for the cds-hooks server to pull further information from the EHR at all, for instance.
I'm not opposed to being able to make - or even recommend - these restrictions (though they need more clarity as Kevin says) but they can't be applied always

@kpshek, yes, I'm thinking of the "read" and "search" interactions. I was trying to describe them in terms of the HTTP level FHIR API only, so that was the reason for the reference to GET, but after reading your updates to the prefetch on the other pull, I agree, and I've removed the references to HTTP verbs.
I've also expanded the Patient request to include resource for which an id is known (so it could include Practitioner or Encounter, for example).
As far as documentation about how these restrictions are used within Clinical Reasoning, there is documentation on the DataRequirement, are you thinking a link there would be helpful?
@grahamegrieve, agreed, I've relaxed the language to make them 'recommended', rather than 'required'.


Readme File adding to Github user search

Describe the Feature you want to end
Would like to document github user search project

Expected behavior
It's helps new contributors to understand features of contribution and steps to do

Screenshots (optional)

Additional context (optional )
I have made many documentations before whichc helps in making this is effective way.
Would like to contribute as GSSOC'23 Contributor.

@MohitGupta121 kindly, assign this to me.


Remove GRC chart

Issue: https://github.com/stolostron/backlog/issues/25585
Signed-off-by: Jakob Gray<EMAIL_ADDRESS>

/lgtm


Machine translation using Seq2Seq with attention by using LSTM instead of GRU

Hello,
I am using the template in tutorial 'Machine translation using Seq2Seq with attention'. However, seq2seq is built on GRU.(The link is https://www.tensorflow.org/tutorials/text/nmt_with_attention)
Could you please provide a template that Machine translation using Seq2Seq with attention using LSTM instead of GRU?

@yanglei-github Please take a look at this doc where LSTM has been user for Neural Machine Translation. Thanks!

Thanks a lot!  Also, do you have some tutorials for implementing copy mechanism(pointer-generator or copynet) codes in TensorFlow version 2? Because I think by far TensorFlow doesn't have a build-in API for using copy mechanism in TensorFlow.

And it is really a great tutorial for machine translation. If it is possible that add some evaluation metrics such as bleu or meteor to evaluate the result

@yanglei-github If its a document feature request can you create a new issue to track this. Thank you!

OK! Sure!


Refactor KFP pipeline "code" generation to produce KFP Python DSL

Is your feature request related to a problem? Please describe.
https://github.com/elyra-ai/elyra/blob/master/elyra/pipeline/processor_kfp.py#L403 produces output that does not look like vanilla KFP Python DSL code, as documented here.
Describe the solution you'd like
Produce KFP Python DSL code that KFP developers are familiar with, not something that seems to be undocumented.

Closed by #2301


[建议] 添加一个后台查询内容记录功能?

自建的被人共享出去被疯狂的跑GPT4,我想知道它们到底在干什么

Hi, the chat history is stored in client side. So you cannot retrieve others chat history. Its possible after we support server-side history sync


[backport] Remove duplicated subtraction in cupy.random.Generator.integers

Backport of #5247

[automatic post] Jenkins, test this please.

Jenkins CI test (for commit 0f3aa3521c5961295ba5b564a23827e0552de4ea, target branch v9) succeeded!


Check whether Alcatraz is installed?

Not necessarily an issue but I was wondering if there's a way to check whether Alcatraz is installed? which -s Alcatraz or which -s Alcatraz.xcplugin doesn't work.

Hi @mattgabor, you can simply check if the Alcatraz.xcplugin exists in Xcode's plugins directory:
test -d ~/Library/Application\ Support/Developer/Shared/Xcode/Plug-ins/Alcatraz.xcplugin

Take note however that it doesn't cover the cases when Alcatraz being installed but not compatible with the current running Xcode version or when Xcode added Alcatraz to the skipped plugins list.
I encourage you to take a look at https://github.com/alcatraz/Alcatraz/blob/master/Scripts/install.sh to understand how we install Alcatraz. Let me know if you need more info.


feat: add remote actor support

Adds support for spawning and messaging actors remotely, built on top of grpc.

👍👍👍


Bad characters in json / API response

There are some bad characters in the data.
E.g., in ICIC.json we find:
  "ICIC0000001": {
    "BANK": "ICICI BANK LIMITED",
    "IFSC": "ICIC0000001",
    "BRANCH": "CHENNAI - CENOTAPH ROAD┬á",
    "ADDRESS": "1, CENOTAPH ROAD, CHENNAI.┬á",
    "CONTACT": null,
    "CITY": "CHENNAI",
    "DISTRICT": "CHENNAI",
    "STATE": "TAMIL NADU"
  },
  "ICIC0000002": {
    "BANK": "ICICI BANK LIMITED",
    "IFSC": "ICIC0000002",
    "BRANCH": "BANGALORE - M G ROAD┬á",
    "ADDRESS": "ICICI BANK TOWERS, 1, COMMISSARIAT ROAD, GROUND FLOOR, BANGALORE┬á560025.┬á",
    "CONTACT": null,
    "CITY": "BANGALORE",
    "DISTRICT": "BANGALORE URBAN",
    "STATE": "KARNATAKA"
  }

These bad characters are also part of the GET API responses.

This is a known issue at RBI end: https://github.com/razorpay/ifsc/issues/29

Please reach out to RBI here: https://www.rbi.org.in/Scripts/helpdesk.aspx


Fragments in policies for cluster execution

Description
Fragments moved to Serving-core. Fill fragments in SparktaClusterJob.
Paths in hdfs with id.
Bug for using TwitterJson in cluster execution corrected.
Mesos execution tested in the old cluster with front.
Test
Some test are moved from serving-api to serving-core

LGTM