refactor to separate EFS from parser #7

hanyelgaml-roche · 2021-07-22T21:29:01Z

Make current package epivizFileServer depend on epivizFileParser

The following errors are fixed FAILED tests/test_measurements.py::test_bigwig_measurement - TypeError: get_data() missing 1 required positional argument: ‘bins’ FAILED tests/test_measurements.py::test_bigbed_measurement - TypeError: get_data() missing 1 required positional argument: ‘bins’

hcorrada

The requirement for efs_parser should be stated on setup.cfg not in the requirements.txt file, check with Jay if efs_parser is already in pypi

hanyelgaml-roche · 2021-07-22T21:50:38Z

The requirement for efs_parser should be stated on setup.cfg not in the requirements.txt file, check with Jay if efs_parser is already in pypi

I checked with Jay and it is not in pypi

requirements.txt

jkanche · 2021-07-22T22:51:51Z

@hanyelgaml-roche couple of things

Package is now on PyPI -https://pypi.org/project/epivizFileParser
package is renamed from efs_parser to epivizFileParser and so is the GitHub repo

Also, use setup.cfg for specifying epivizFileParser as a dependancy

setup.cfg

src/epivizfileserver/__init__.py

src/epivizfileserver/handler/utils.py

src/epivizfileserver/measurements/measurementManager.py

src/epivizfileserver/server/utils.py

tests/test_parser.py

setup.cfg

jkanche · 2021-07-23T20:25:59Z

@hanyelgaml-roche tests need to be written for the handler (dask part) to make sure its not failing because of this change.

you have written tests here and they can be brought over - epiviz/efs_distributed#1

jkanche · 2021-07-26T03:40:12Z

@hanyelgaml-roche did you run tests here ?

python setup.py test ????

hanyelgaml-roche · 2021-07-26T13:02:26Z

@hanyelgaml-roche did you run tests here ?

python setup.py test ????

Of Couese

platform darwin -- Python 3.7.9, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 -- /Users/elgamlh/epivizFileServer_fork/venv/bin/python
cachedir: .pytest_cache
rootdir: /Users/elgamlh/epivizFileServer_fork, configfile: setup.cfg, testpaths: tests
plugins: cov-2.12.1
collected 4 items                                                                                                                                                                                                                      

tests/test_measurements.py::test_bigwig_measurement PASSED                                                                                                                                                                       [ 25%]
tests/test_measurements.py::test_bigbed_measurement PASSED                                                                                                                                                                       [ 50%]
tests/test_parser.py::test_bigwig PASSED                                                                                                                                                                                         [ 75%]
tests/test_parser.py::test_bigbed PASSED                                                                                                                                                                                         [100%]

=========================================================================================================== warnings summary ===========================================================================================================
venv/lib/python3.7/site-packages/sanic/base.py:40
  /Users/elgamlh/epivizFileServer_fork/venv/lib/python3.7/site-packages/sanic/base.py:40: DeprecationWarning: Sanic instance named 'epivizfileserver.server' uses a format that isdeprecated. Starting in version 21.12, Sanic objects must be named only using alphanumeric characters, _, or -.
    DeprecationWarning,

venv/lib/python3.7/site-packages/sanic/base.py:64
  /Users/elgamlh/epivizFileServer_fork/venv/lib/python3.7/site-packages/sanic/base.py:64: DeprecationWarning: Setting variables on Sanic instances is deprecated and will be removed in version 21.9. You should change your Sanic instance to use instance.ctx.epivizFileHandler instead.
    DeprecationWarning,

tests/test_measurements.py::test_bigwig_measurement
  /Users/elgamlh/epivizFileServer_fork/tests/test_measurements.py:60: RuntimeWarning: coroutine 'FileMeasurement.get_data' was never awaited
    assert mMgr.get_measurement('test-bw').get_data("1", 1, 1000,bins=100)

tests/test_measurements.py::test_bigbed_measurement
  /Users/elgamlh/epivizFileServer_fork/tests/test_measurements.py:63: RuntimeWarning: coroutine 'FileMeasurement.get_data' was never awaited
    assert mMgr.get_measurement('test-bb').get_data("chr1", 1, 1000,bins=100)

-- Docs: https://docs.pytest.org/en/stable/warnings.html

---------- coverage: platform darwin, python 3.7.9-final-0 -----------
Name                                                      Stmts   Miss Branch BrPart  Cover   Missing
-----------------------------------------------------------------------------------------------------
src/epivizfileserver/__init__.py                             12      2      0      0    83%   13-14
src/epivizfileserver/cli.py                                 103    103     36      0     0%   17-214
src/epivizfileserver/client/EpivizClient.py                  28     28      2      0     0%   1-74
src/epivizfileserver/client/__init__.py                       1      1      0      0     0%   1
src/epivizfileserver/handler/HandlerNoActor.py               83     83     12      0     0%   2-163
src/epivizfileserver/handler/__init__.py                      1      0      0      0   100%
src/epivizfileserver/handler/handler.py                     125    100     22      0    17%   20-32, 47-52, 66-68, 80-101, 106-111, 119-130, 133-137, 140-146, 149-161, 175-187, 200-209, 215-218
src/epivizfileserver/handler/utils.py                         4      2      0      0    50%   14-38
src/epivizfileserver/measurements/__init__.py                 2      0      0      0   100%
src/epivizfileserver/measurements/measurementClass.py       274    209     76      1    19%   51, 64, 70, 75, 80, 85, 90, 95, 100, 105, 110, 115, 120-122, 127, 132, 137-141, 157-183, 186-198, 207, 222-225, 237-239, 253-294, 323-324, 338-352, 367-402, 405-411, 425-429, 432-435, 447-455, 471-507, 516-518, 533-586
src/epivizfileserver/measurements/measurementManager.py     344    287    118      1    13%   19-26, 29-33, 36-46, 53-67, 70-82, 85-97, 100-113, 116-145, 148-167, 170-184, 194-197, 203, 206, 237-256, 266-270, 282-374, 380-390, 393-398, 424-431, 441-485, 494-497, 500, 510-511, 530-532, 543-601, 606-613, 620-623, 629, 638-646
src/epivizfileserver/server/__init__.py                     131     99     28      0    20%   46-57, 69-74, 84-87, 91-114, 119-134, 155-166, 190-200, 222, 235-268, 273-312
src/epivizfileserver/server/request.py                      207    174     66      0    12%   28-38, 46-48, 57, 67, 75-77, 80, 84-92, 99-100, 103, 107-145, 152-153, 156-183, 187-280, 287-288, 291-300, 303-323, 327-328, 331-349
src/epivizfileserver/server/utils.py                         66     60     36      0     6%   17-34, 49-133, 147-173
src/epivizfileserver/trackhub/TrackHub.py                    96     88     50      0     5%   14-18, 21-42, 45-74, 77-143, 153-176
src/epivizfileserver/trackhub/__init__.py                     1      0      0      0   100%
-----------------------------------------------------------------------------------------------------
TOTAL                                                      1478   1236    446      2    13%

==================================================================================================== 4 passed, 4 warnings in 5.16s =====================================================================================================
(v
```

jkanche · 2021-07-26T13:51:46Z

if you change these lines in the tests to

from epivizFileParser.BigBed import BigBed
from epivizFileParser.BigWig import BigWig

to how the parser is imported

from epivizFileParser import BigWig, BigBed

everything fails. So the imports need to be consistent between the tests and its usage in the package

hcorrada · 2021-07-26T16:07:31Z

Hi @jkanche ,

For this to work:

from epivizFileParser import BigWig, BigBed

the parser package needs to export those classes, which it currently does not.

I suggested to Hany that he should modify https://github.com/epiviz/epivizFileParser/blob/main/src/epivizFileParser/__init__.py

to include, e.g.,

from .BigWig import BigWig

for all the classes we want to expose.
Please confirm.

hanyelgaml-roche · 2021-07-26T16:31:21Z

from epivizFileParser import BigWig, BigBed

I have fixed that on epivizFileParser
and created a PR:
epiviz/epivizFileParser#8

jkanche · 2021-07-26T20:21:27Z

Thanks @hanyelgaml-roche this is now merged with master

hanyelgaml-roche added 3 commits July 22, 2021 17:01

Remove parser directory and use efs_parser package

bb9de5c

Add efs_parser wheel file to requirements.txt

78c8218

hcorrada reviewed Jul 22, 2021

View reviewed changes

Delete efs_parser wheel file and install package from github

55fd02b

jkanche self-requested a review July 22, 2021 22:21

jkanche assigned hanyelgaml-roche Jul 22, 2021

jkanche requested changes Jul 22, 2021

View reviewed changes

requirements.txt Outdated Show resolved Hide resolved

Rename package efs_parser to its PyPi name epivizFileParser

3177f73

Also, use setup.cfg for specifying epivizFileParser as a dependancy

jkanche requested changes Jul 22, 2021

View reviewed changes

jkanche changed the title ~~Clear separation~~ refactor to separate EFS from parser Jul 22, 2021

Remove commented code

7e158b8

jkanche reviewed Jul 23, 2021

View reviewed changes

setup.cfg Outdated Show resolved Hide resolved

Remove wheel from dependancy

7f573fb

jkanche closed this Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor to separate EFS from parser #7

refactor to separate EFS from parser #7

hanyelgaml-roche commented Jul 22, 2021

hcorrada left a comment •

edited

Loading

hanyelgaml-roche commented Jul 22, 2021

jkanche commented Jul 22, 2021 •

edited

Loading

jkanche commented Jul 23, 2021 •

edited

Loading

jkanche commented Jul 26, 2021

hanyelgaml-roche commented Jul 26, 2021

jkanche commented Jul 26, 2021

hcorrada commented Jul 26, 2021

hanyelgaml-roche commented Jul 26, 2021

jkanche commented Jul 26, 2021

refactor to separate EFS from parser #7

refactor to separate EFS from parser #7

Conversation

hanyelgaml-roche commented Jul 22, 2021

hcorrada left a comment • edited Loading

Choose a reason for hiding this comment

hanyelgaml-roche commented Jul 22, 2021

jkanche commented Jul 22, 2021 • edited Loading

jkanche commented Jul 23, 2021 • edited Loading

jkanche commented Jul 26, 2021

hanyelgaml-roche commented Jul 26, 2021

jkanche commented Jul 26, 2021

hcorrada commented Jul 26, 2021

hanyelgaml-roche commented Jul 26, 2021

jkanche commented Jul 26, 2021

hcorrada left a comment •

edited

Loading

jkanche commented Jul 22, 2021 •

edited

Loading

jkanche commented Jul 23, 2021 •

edited

Loading