dffml: source: file: Add support for .zip file #38

sudharsana-kjl · 2019-03-25T09:55:19Z

The zipfile module has been used to support .zip files

codecov-io · 2019-03-25T09:57:20Z

Codecov Report

Merging #38 into master will decrease coverage by 0.34%.
The diff coverage is 48.14%.

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
- Coverage   89.29%   88.95%   -0.35%     
==========================================
  Files          46       46              
  Lines        3186     3213      +27     
  Branches      334      337       +3     
==========================================
+ Hits         2845     2858      +13     
- Misses        291      303      +12     
- Partials       50       52       +2

Impacted Files	Coverage Δ
tests/source/test_file.py	`100% <100%> (ø)`	⬆️
dffml/source/file.py	`79.1% <17.64%> (-20.9%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf3493e...187ea5a. Read the comment docs.

yashlamba · 2019-03-25T10:02:37Z

I guess you won't need a text wrapper, ZipFile defaults to text type. I was actually working on it since morning and was going to open a PR. The issue I am facing is it isn't working with iris_training case, did you try the same?

EDIT: My bad, got the text wrapper.

sudharsana-kjl · 2019-03-25T10:08:27Z

Without text wrapper, it throws the following error:

Traceback (most recent call last):
  File "/home/sudharsana/GSoC/dffml/env/bin/dffml", line 11, in <module>
    load_entry_point('dffml==0.1.1', 'console_scripts', 'dffml')()
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/cli.py", line 145, in main
  File "/usr/lib/python3.6/asyncio/base_events.py", line 484, in run_until_complete
    return future.result()
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/cli.py", line 127, in cli
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/cli.py", line 203, in run
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/asynchelper.py", line 20, in __aenter__
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/source.py", line 67, in __aenter__
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/file.py", line 39, in open
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/file.py", line 60, in _open
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/csvfile.py", line 35, in load_fd
  File "/usr/lib/python3.6/csv.py", line 111, in __next__
    self.fieldnames
  File "/usr/lib/python3.6/csv.py", line 98, in fieldnames
    self._fieldnames = next(self.reader)
_csv.Error: iterator should return strings, not bytes (did you open the file in text mode?)

This is why i used text wrapper

sudharsana-kjl · 2019-03-25T10:10:08Z

@yashlamba To fix your error, make sure that you change the headers using sed -i 's/.*setosa,versicolor,virginica/SepalLength,SepalWidth,PetalLength,PetalWidth,classification/g' *.csv and then compress it. Add the text wrapper as well.

sudharsana-kjl · 2019-03-25T10:24:54Z

Also, this works only for a zip file that contains a single file inside.

dffml/dffml/source/file.py

Lines 55 to 59 in 32389f3

    
           elif zipfile.is_zipfile(self.filename): 
        
               archive = zipfile.ZipFile(self.filename) 
        
               for file in archive.infolist(): 
        
                   file = io.TextIOWrapper(archive.open(file, 'r')) 
        
                   await self.load_fd(file)

Since a zip folder can contain multiple files, when I try to load them all inside the for-loop, only the last file in the archive.infolist() is getting added to the memory.

I think this happens because of:

dffml/dffml/source/csvfile.py

Lines 29 to 34 in 32389f3

    
               async def load_fd(self, fd): 
        
                   ''' 
        
                   Parses a CSV stream into Repo instances 
        
                   ''' 
        
                   i = 0 
        
                   self.mem = {}

self.mem={} in the beginning of every call to load_fd() and I'm trying to operate on a single source object. so its getting overwritten.

Test

Test cases havent been included yet. @pdxjohnny any alternative to using FakeFileSource in tests? Because I use the is_zipfile() to check whether its a zip file or not. So the usual file extension check cant be used here. Can we import zipfile package and create a new zipfile inside test?

EDIT: After having a discussion with Yash, we have decided that I'll be continuing to add the test cases as well.

sudharsana-kjl · 2019-03-25T10:25:51Z

Without text wrapper, it throws the following error:

Traceback (most recent call last):
  File "/home/sudharsana/GSoC/dffml/env/bin/dffml", line 11, in <module>
    load_entry_point('dffml==0.1.1', 'console_scripts', 'dffml')()
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/cli.py", line 145, in main
  File "/usr/lib/python3.6/asyncio/base_events.py", line 484, in run_until_complete
    return future.result()
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/cli.py", line 127, in cli
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/cli.py", line 203, in run
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/asynchelper.py", line 20, in __aenter__
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/source.py", line 67, in __aenter__
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/file.py", line 39, in open
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/file.py", line 60, in _open
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/csvfile.py", line 35, in load_fd
  File "/usr/lib/python3.6/csv.py", line 111, in __next__
    self.fieldnames
  File "/usr/lib/python3.6/csv.py", line 98, in fieldnames
    self._fieldnames = next(self.reader)
_csv.Error: iterator should return strings, not bytes (did you open the file in text mode?)

This is why i used text wrapper

@yashlamba were you able to resolve the error?

yashlamba · 2019-03-25T10:29:55Z

Just wondering, why can't we use the regular file extension check? I tried and it seems to work fine.

yashlamba · 2019-03-25T10:33:27Z

Without text wrapper, it throws the following error:

Traceback (most recent call last):
  File "/home/sudharsana/GSoC/dffml/env/bin/dffml", line 11, in <module>
    load_entry_point('dffml==0.1.1', 'console_scripts', 'dffml')()
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/cli.py", line 145, in main
  File "/usr/lib/python3.6/asyncio/base_events.py", line 484, in run_until_complete
    return future.result()
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/cli.py", line 127, in cli
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/cli.py", line 203, in run
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/util/asynchelper.py", line 20, in __aenter__
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/source.py", line 67, in __aenter__
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/file.py", line 39, in open
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/file.py", line 60, in _open
  File "/home/sudharsana/GSoC/dffml/env/lib/python3.6/site-packages/dffml-0.1.1-py3.6.egg/dffml/source/csvfile.py", line 35, in load_fd
  File "/usr/lib/python3.6/csv.py", line 111, in __next__
    self.fieldnames
  File "/usr/lib/python3.6/csv.py", line 98, in fieldnames
    self._fieldnames = next(self.reader)
_csv.Error: iterator should return strings, not bytes (did you open the file in text mode?)

This is why i used text wrapper

@yashlamba were you able to resolve the error

It was happening with my code actually, I didn't try yours then.

It's working great with yours!

sudharsana-kjl · 2019-03-25T11:26:35Z

@yashlamba just checking the file extension works fine in most cases but it is not the best possible way. Any file extension can be modified. So I felt a more strict way is using the available functions to check them.

johnandersen777 · 2019-03-30T00:23:57Z

dffml/source/file.py

+        elif zipfile.is_zipfile(self.filename):
+            archive = zipfile.ZipFile(self.filename)
+            for file in archive.infolist():
+                file = io.TextIOWrapper(archive.open(file, 'r'))


TextIOWrapper inherits from https://docs.python.org/3/library/io.html#io.IOBase io.IOBase so we can use the with keyword here as well.

Hence we have the opportunity to keep the convention of using opener = something.

However, since we'll use the with keyword just on opener on line 65, we need to define a little helper function so that both the zipfile and the textiowrapper are closed at the end of the with block.

from contextlib import contextmanager ... elif zipfile.is_zipfile(self.filename): @contextmanager def opener_helper(): with zipfile.ZipFile(self.filename) as archive: for file in archive.infolist(): with io.TextIOWrapper(archive.open(filename, 'r')) as fd: yield fd # Only care about the one file break opener = opener_helper()

johnandersen777 · 2019-03-30T00:27:31Z

dffml/source/file.py

@@ -66,10 +77,19 @@ def __repr__(self):
            elif self.filename[::-1].startswith(('.xz')[::-1]) or \
                    self.filename[::-1].startswith(('.lzma')[::-1]):
                close = lzma.open(self.filename, 'wt')
+            elif zipfile.is_zipfile(self.filename):
+                tmp = tempfile.NamedTemporaryFile(suffix='.csv')


lets avoid the use of a tempfile here. I think a helper method like with load might be useful, see https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.write or maybe https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.writestr

I have added a helper method similar to the one in _open method.

sudharsana-kjl · 2019-03-31T00:00:42Z

@pdxjohnny Could you suggest how to write test cases for this feature?

Right now, a FakeFileSource object is created in the test. Since most of the other file type support only check the extension, this works fine. But here I've used an inbuilt function is_zipfile() to check. Because of this, I wont be able to use the existing FakeFileSource class.

johnandersen777 · 2019-03-31T00:44:45Z

I think you can mock the return_value of is_zipfile to return True, then check that it tries to open. I think you may need to mock return_value's for textiowrapper too, but not sure

sudharsana-kjl · 2019-04-01T02:48:28Z

The usage of zipfile.is_zipfile() throws the following error while running existing tests:

(env) sudharsana@sudharsana-HP-15-Notebook-PC:~/GSoC/dffml(zipfile_feature)$ python -m unittest discover
.E.......E......................
add 40 and 2 {
    "get_single":{
        "result":42
    }
}


multiply 42 and 10 {
    "get_single":{
        "result":420
    }
}

............................................................
..........................
======================================================================
ERROR: test_close (tests.source.test_file.TestFileSource)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sudharsana/GSoC/dffml/dffml/util/asynctestcase.py", line 39, in run_it
    result = self.loop.run_until_complete(coro(*args, **kwargs))
  File "/usr/lib/python3.7/asyncio/base_events.py", line 584, in run_until_complete
    return future.result()
  File "/home/sudharsana/GSoC/dffml/tests/source/test_file.py", line 108, in test_close
    await source.close()
  File "/home/sudharsana/GSoC/dffml/dffml/source/file.py", line 71, in close
    await asyncio.shield(self._close())
  File "/home/sudharsana/GSoC/dffml/dffml/source/file.py", line 82, in _close
    elif zipfile.is_zipfile(self.filename):
  File "/usr/lib/python3.7/zipfile.py", line 204, in is_zipfile
    result = _check_zipfile(fp)
  File "/usr/lib/python3.7/zipfile.py", line 187, in _check_zipfile
    if _EndRecData(fp):
  File "/usr/lib/python3.7/zipfile.py", line 289, in _EndRecData
    maxCommentStart = max(filesize - (1 << 16) - sizeEndCentDir, 0)
TypeError: '>' not supported between instances of 'int' and 'MagicMock'

======================================================================
ERROR: test_open (tests.source.test_file.TestFileSource)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sudharsana/GSoC/dffml/dffml/util/asynctestcase.py", line 39, in run_it
    result = self.loop.run_until_complete(coro(*args, **kwargs))
  File "/usr/lib/python3.7/asyncio/base_events.py", line 584, in run_until_complete
    return future.result()
  File "/home/sudharsana/GSoC/dffml/tests/source/test_file.py", line 63, in test_open
    await source.open()
  File "/home/sudharsana/GSoC/dffml/dffml/source/file.py", line 39, in open
    await asyncio.shield(self._open())
  File "/home/sudharsana/GSoC/dffml/dffml/source/file.py", line 55, in _open
    elif zipfile.is_zipfile(self.filename):
  File "/usr/lib/python3.7/zipfile.py", line 204, in is_zipfile
    result = _check_zipfile(fp)
  File "/usr/lib/python3.7/zipfile.py", line 187, in _check_zipfile
    if _EndRecData(fp):
  File "/usr/lib/python3.7/zipfile.py", line 289, in _EndRecData
    maxCommentStart = max(filesize - (1 << 16) - sizeEndCentDir, 0)
TypeError: '>' not supported between instances of 'int' and 'MagicMock'

----------------------------------------------------------------------
Ran 118 tests in 0.685s

FAILED (errors=2)

Because of this, I have changed the condition to check only the extension similar to other file types.

Also, I tried using FakeFileSource object but certain inbuilt functions in zipfile was different which required specific attributes. So, I created a MockZipFile class in test_file.

johnandersen777

This is looking great! Thanks for all your work on this, I think these are my final comments on this.

johnandersen777 · 2019-04-05T13:58:10Z

tests/source/test_file.py

+        source.close = m_open
+        with patch('os.path.exists', return_value=True), \
+                patch('zipfile.ZipFile',m_open):
+            source.close()


The await keyword needs to be used in front of any async functions

Travis:

test_close_xz (tests.source.test_file.TestFileSource) ... ok test_close_zip (tests.source.test_file.TestFileSource) ... /home/travis/build/intel/dffml/tests/source/test_file.py:153: RuntimeWarning: coroutine 'FileSource.close' was never awaited source.close() ok test_filename (tests.source.test_file.TestFileSource) ... ok test_filename_readonly (tests.source.test_file.TestFileSource) ... ok ... ok test_open_xz (tests.source.test_file.TestFileSource) ... ok test_open_zip (tests.source.test_file.TestFileSource) ... /home/travis/build/intel/dffml/tests/source/test_file.py:104: RuntimeWarning: coroutine 'FileSource.open' was never awaited source.open() ok

using await throws the following error:
TypeError: object MagicMock can't be used in 'await' expression

Initially I was using a MockZipFile object. I changed the approach to using FakeFileSource object similar to how it is done already. Even then the error persists.

Hm, okay I'll check it out

dffml/source/file.py

Signed-off-by: John Andersen <john.s.andersen@intel.com>

…o zipfile_feature

johnandersen777 · 2019-04-11T22:10:16Z

Thanks for your work on this @sudharsana-kjl! After trying my hand at this I see how confusing zipfile is! I had to move things around a bit.

sudharsana-kjl · 2019-04-12T01:54:36Z

@pdxjohnny I was looking at the commit a58acb6. This is great! dumping the data back as bzip makes it less complex. Also in tests, the use of yield_42 function is a great idea. Also, I have a doubt, since MockZipFile object is not used anymore, could you please remove it? or do you have any future plans of using it?

johnandersen777 · 2019-04-12T15:37:44Z

Damn! Good catch. Ideally we figure out how to test these lines: https://codecov.io/gh/intel/dffml/src/master/dffml/source/file.py#L83...94

and that probably still involves using the MockZipFile. If you want to fix my error there I'd appreciate it, it would also mean getting rid of yield42 (it think).

add support for zip file

32389f3

johnandersen777 suggested changes Mar 30, 2019

View reviewed changes

sudharsana-kjl added 4 commits March 30, 2019 06:56

fix merge conflict

d7ac0ff

modify _open

d1e1577

modify _close

59f869c

remove unnecessary import

2d2f12f

add test for zip file feature

ee42a23

Merge remote-tracking branch 'upstream/master' into zipfile_feature

02b0763

johnandersen777 suggested changes Apr 5, 2019

View reviewed changes

sudharsana-kjl and others added 6 commits April 10, 2019 21:33

remove hard coded csv filetype in zipfile dump

980ea90

dump innerfile in zipfile without extension

a34ed50

modify test_file

6cc3182

remove extra. in filename for zipfile dump

187ea5a

Changes test for file

a58acb6

Signed-off-by: John Andersen <john.s.andersen@intel.com>

Merge branch 'zipfile_feature' of github.com:sudharsana-kjl/dffml int…

c28223c

…o zipfile_feature

johnandersen777 merged commit 1a66085 into intel:master Apr 11, 2019

johnandersen777 mentioned this pull request May 21, 2019

source: file: Compression #15

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dffml: source: file: Add support for .zip file #38

dffml: source: file: Add support for .zip file #38

sudharsana-kjl commented Mar 25, 2019

codecov-io commented Mar 25, 2019 •

edited

Loading

yashlamba commented Mar 25, 2019 •

edited

Loading

sudharsana-kjl commented Mar 25, 2019

sudharsana-kjl commented Mar 25, 2019 •

edited

Loading

sudharsana-kjl commented Mar 25, 2019 •

edited

Loading

sudharsana-kjl commented Mar 25, 2019

yashlamba commented Mar 25, 2019

yashlamba commented Mar 25, 2019

sudharsana-kjl commented Mar 25, 2019 •

edited

Loading

johnandersen777 Mar 30, 2019

sudharsana-kjl Mar 30, 2019

johnandersen777 Mar 30, 2019

sudharsana-kjl Mar 30, 2019

sudharsana-kjl commented Mar 31, 2019

johnandersen777 commented Mar 31, 2019

sudharsana-kjl commented Apr 1, 2019

johnandersen777 left a comment

johnandersen777 Apr 5, 2019

johnandersen777 Apr 10, 2019

sudharsana-kjl Apr 10, 2019

sudharsana-kjl Apr 10, 2019

johnandersen777 Apr 10, 2019

johnandersen777 commented Apr 11, 2019

sudharsana-kjl commented Apr 12, 2019

johnandersen777 commented Apr 12, 2019

dffml: source: file: Add support for .zip file #38

dffml: source: file: Add support for .zip file #38

Conversation

sudharsana-kjl commented Mar 25, 2019

codecov-io commented Mar 25, 2019 • edited Loading

Codecov Report

yashlamba commented Mar 25, 2019 • edited Loading

sudharsana-kjl commented Mar 25, 2019

sudharsana-kjl commented Mar 25, 2019 • edited Loading

sudharsana-kjl commented Mar 25, 2019 • edited Loading

Test

sudharsana-kjl commented Mar 25, 2019

yashlamba commented Mar 25, 2019

yashlamba commented Mar 25, 2019

sudharsana-kjl commented Mar 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudharsana-kjl commented Mar 31, 2019

johnandersen777 commented Mar 31, 2019

sudharsana-kjl commented Apr 1, 2019

johnandersen777 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnandersen777 commented Apr 11, 2019

sudharsana-kjl commented Apr 12, 2019

johnandersen777 commented Apr 12, 2019

codecov-io commented Mar 25, 2019 •

edited

Loading

yashlamba commented Mar 25, 2019 •

edited

Loading

sudharsana-kjl commented Mar 25, 2019 •

edited

Loading

sudharsana-kjl commented Mar 25, 2019 •

edited

Loading

sudharsana-kjl commented Mar 25, 2019 •

edited

Loading