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

Add mcool datatype and extend h5 tool output testing #7993

Merged
merged 11 commits into from
May 18, 2019

Conversation

msauria
Copy link
Contributor

@msauria msauria commented May 16, 2019

This PR does two things.

  1. It adds a datatype for a multi-resolution cooler file (the cooler datatype is already in main).
  2. It also adds a new assertion element for testing tool output. Currently, h5 output files can be tested by looking at attributes name and values, and keys, but only in the base group. This new test, 'contains_h5_keys' would allow specification of a subset of keys (currently the passed set of keys must exactly match all keys in the output) and allow drilling into groups so multilayered h5 files could be tested (e.g. 'base/subkey').

@galaxybot galaxybot added this to the 19.09 milestone May 16, 2019
Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @msauria. Small review inline.

ping @joachimwolff

lib/galaxy/web/proxy/js/lib/proxy.js Outdated Show resolved Hide resolved
test/functional/tools/validation_hdf5.xml Show resolved Hide resolved
@@ -1652,6 +1652,8 @@ module.
</xs:element>
<xs:element name="has_h5_keys" type="AssertHasH5Keys">
</xs:element>
<xs:element name="contains_h5_keys" type="AssertContainsH5Keys">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_ and contains_ is for a user probably the same. Could we extend the current has_ functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose so. I wanted to avoid needing to specify a large number of keys for complex h5 files, but given that this is only invoked for tests, I suppose test output can be controlled to keep this list reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do you mean altering the 'has_h5_keys' test? Doing so would definitely break any tests currently using it if they have groups in the h5 files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reading this as change has_h5_keys to not require a complete exact match. Which makes sense, "has" does not mean "has only".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @jxtx said is what I wanted to say :) Sorry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

lib/galaxy/datatypes/binary.py Show resolved Hide resolved
Removed log statements from proxy.js
Added space back to validation_hdf5.xml
Changed Mcool class to inherit from Cool in binary.py
>>> fname = get_test_fname('wiggle.wig')
>>> MCool().sniff(fname)
False
>>> fname = get_test_fname('biom2_sparse_otu_table_hdf5.biom2')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add here a cool test what will eval to False?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Done

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Mike!

@bgruening bgruening merged commit b8f9d9a into galaxyproject:dev May 18, 2019
@galaxybot
Copy link
Contributor

This PR was merged without a 'kind/' tag, please correct.

@bgruening bgruening changed the title Add mcool datatypeand extend h5 tool output testing Add mcool datatype and extend h5 tool output testing May 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants