Parquet_integration

[diskontinuum]

Includes:

(1) Option to handle very large data sets by writing all .CSV files (per compartment) to a Parquet File -> issue #96 .

(2) determination of a reference table (via sampling or with a specified folder) -> issue #100 (missing columns)

(3) alignment w.r.t. reference table -> partly addresses issue #91, because there is no ingestion error for missing columns and zero-valued columns. However, the table is still read (and missing values are set to NaN)
(3) type conversion to deal with type inconsistencies (necessary to be able to write to same Parquet file)

(4) Several functions that return a dictionary of table paths of a certain table type. This needs work. Ideally, we condense all of them to a single general function which also solves issue #88.

Needs Work:

• eliminate separate processing of image.CSV v.s. CSV files of compartments (so much redundant code and many potential bugs due to the inconsistent data structures).
• transfer testing functions from colaboratory notebooks
• add Pytest test scripts: (1) Modification of old test_ingest.py that runs with Parquet option (comparing the shape of files) and (2) Pytest script that also includes comparison of tabular values (modify from colaboratory notebooks).
• condense functions of similar functionality
• find and delete unused functions and change/delete hard-coded parts s.t. everything can be read from config.ini, consistently. This addresses almost all functions in utils.py.

Note:

• .CSV tables from different wells are now in the same Parquet File, but distinguishable as distinct "row-group". The different table types are in different Parquet Files (e.g. image.parquet, Cell.parquet, ...)
• An obvious and easy next step is to write a function that concatenates (horizontally) the Parquet files of different types. We can simply merge tables according to their (identical) TableNumber.

[gwaybio]

thanks for filing the PR @diskontinuum - it is clear this represents a ton of work and its great you're bringing the project to the next level. It is not quite there yet - we can iterate on this pull request until we get there.

First, before I perform a more in depth review, we need to get the testing suite figured out. In general, travis has a really nice interface to figure out exactly where problems arise. (after clicking on the details link above next to the continuous-integration/travis-ci/ build) you will notice some strange error on line 350 (https://travis-ci.org/cytomining/cytominer-database/jobs/650395552?utm_medium=notification&utm_source=github_status)

KeyError: PosixPath('/home/travis/build/cytomining/cytominer-database/tests/conftest.py')

which is related to the following error on line 425

  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/_pytest/config/argparsing.py", line 314, in addoption
    raise ValueError("option names %s already added" % conflict)

Old Behavior

We don't see this testing error in new buildes (example here).

Recommended Solution

Part 1

It is quite difficult at the moment to parse through exactly what change is causing this error. I noticed that you've created a new folder structure called tests_WIP. I think now is time to merge that folder into the real testing suite. This will help immensely for viewing exact changes you've made (e.g. there is no quick way to tell if tests_WIP/conftest.py is different than tests/conftest.py.

Part 2

While you are merging, it will also be good to consider how you will drop most of all of the additional data files added in this PR (225 files! 😱 ). I understand the reason for the data files (they're named appropriately for their purpose - which is great), but we should try our best to keep the data files consistent (and not add many different versions with slightly different inputs). Instead of removing a column or inserting an NA in the testing data by copying and manually removing for each new test, load existing data and drop the columns or inject NA entries after loading. This reduces bloat and is more transparent to the software engineer reading the code

Thanks again for the very thoroughly documented code! I am looking forward to jumping in after we've addressed these two important points. Addressing these two points will save a ton of time for PR review, but also for all future cytominer-database developments (of which you have become a major contributor)

[gwaybio]

Great! I see that most files were redundant. One potential concern though - do we keep track of all of those specific data files and which tests were using which data (and the specific manipulations that necessitated data folders in the first place)? (sorry if this is clear in the updated code, I haven't given it a detailed look just yet)

Also, based on the travis build error:

==================================== ERRORS ====================================
____________________ ERROR collecting tests/test_ingest.py _____________________
ImportError while importing test module '/home/travis/build/cytomining/cytominer-database/tests/test_ingest.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tests/test_ingest.py:7: in <module>
    import cytominer_database.ingest
cytominer_database/ingest.py:92: in <module>
    import pyarrow
E   ImportError: No module named 'pyarrow'

It looks pyarrow should also be added to a file called requirements.txt.

I'll also copy here what we discussed to be the next steps for getting this merged in and your contributions realized.

so, to proceed lets

1. Get pytest running locally
2. Make sure the collab code is following pytest conventions (the examples in the repo are a great place to start)
3. Let me know if there are any specific questions

[gwaybio]

Congrats for this huge contribution - version 0.4 here we come!

Please do think about the remaining comments. Once we touch base about them and finalize their discussions, I will merge and initiate the new version release. Thanks for sticking with this!

[gwaybio]

this comment still needs addressing