Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further tighten up GG format. #3233

Merged
merged 1 commit into from Dec 12, 2016

Conversation

Projects
None yet
3 participants
@jmchilton
Copy link
Member

commented Nov 30, 2016

I encountered similar input problems to #864 - this helps a little.

[float(x) for x in row] # first col has been removed
except:
[float(x) for x in rest_row] # first col has been removed
except Exception:

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Nov 30, 2016

Member

s/Exception/ValueError/ ?

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

What about adding a test case in sniff.py?

@jmchilton jmchilton force-pushed the jmchilton:gg_datatype branch from bbe424e to 92f7813 Dec 2, 2016

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 2, 2016

@nsoranzo I believe I have rebased this to address your issues.

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Dec 2, 2016

Thanks @jmchilton, unfortunately a unit test is not passing because there is no check that there is a second line in the file.

@jmchilton jmchilton force-pushed the jmchilton:gg_datatype branch from 92f7813 to 6c7b2fa Dec 8, 2016

buf = f.read(1024)

if len(rows) < 2:
return False

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Dec 8, 2016

Member

These lines should be after you define rows 😉

This comment has been minimized.

Copy link
@jmchilton

jmchilton Dec 8, 2016

Author Member

Lies

@jmchilton jmchilton force-pushed the jmchilton:gg_datatype branch from 6c7b2fa to e281b18 Dec 8, 2016


rows = [l.split() for l in buf.splitlines()[1:4]] # break on lines and drop header, small sample

if len(rows) < 2:

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Dec 8, 2016

Member

Strictly speaking, I think here it should be 1 instead of 2, since we have already skipped the header. Sorry I haven't noticed this on the previous iteration.

This comment has been minimized.

Copy link
@jmchilton

jmchilton Dec 12, 2016

Author Member

Rebased with this change - thanks!

Further tighten up GG format.
I encountered similar input problems to #864 - this helps a little.

- Ensure first column is a valid marker as outlined on https://genome.ucsc.edu/goldenpath/help/hgGenomeHelp.html.
- Avoid readlines() since it may allocate huge amounts on memory on certain inputs.
- Eliminate bare exception.

Rebased with bug fixes in original commit and better testing and tighter exception handling as suggested by @nsoranzo.

@jmchilton jmchilton force-pushed the jmchilton:gg_datatype branch from e281b18 to 96bd455 Dec 12, 2016

@dannon dannon merged commit 176d4d5 into galaxyproject:dev Dec 12, 2016

4 checks passed

api test Build finished. 243 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 131 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.