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

Remove wchartype dependency and is_multi_byte [2/2] #5088

Merged
merged 4 commits into from Nov 30, 2017

Conversation

Projects
None yet
3 participants
@nsoranzo
Member

nsoranzo commented Nov 29, 2017

Together with a bunch of datatype fixes and upload enhancements. See individual commits for details.

xref. #1715

nsoranzo added some commits Nov 28, 2017

Fix some datatype issues
- dmnd had a wrong class name
- biom1, cool and postgresql shouldn't have `subclass="true"`
- extension should be lowercase
- add missing file_ext attribute to various datatype classes
Fix append_to_sniff_order()
`isinstance()` was leaving out e.g. the BigWig datatype because BigBed,
which was added first, is a subclass of BigWig.
Instead, add a datatype only if it is not already in `sniff_order`, it
has a `sniff()` method and was not defined with `subclass="true"`.
Remove is_multi_byte from upload.py
Also, when sniffing binary files, sniff images together with the other
formats and respect sniff order.

Also remove `is_multi_byte` from:
- `stream_to_open_named_file()`
- `stream_to_file()`

Remove the now unused `get_image_ext()` and `Binary.is_sniffable_binary()`
and all the calls to `Binary.register_sniffable_binary_format()`.
@jmchilton

This comment has been minimized.

Member

jmchilton commented Nov 30, 2017

I'll be honest - I'm super embarrassed I didn't do this when I added the ability to add new binary sniffable types. This is clearly much, much cleaner.

@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Nov 30, 2017

I have ready another commit to also remove the unsniffable binary list from Binary, but I'll open another PR tomorrow.

@bgruening

This comment has been minimized.

Member

bgruening commented Nov 30, 2017

Very nice cleanup! Thanks @nsoranzo!

@bgruening bgruening merged commit f70e6f5 into galaxyproject:dev Nov 30, 2017

7 checks passed

api test Build finished. 331 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 164 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 59 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
selenium test Build finished. 114 tests run, 2 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details

@nsoranzo nsoranzo deleted the nsoranzo:remove_wchartype_dep branch Dec 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment