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.

Sign up for GitHub

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

Jump to bottom

ENH: Metadata-compatible manifest formats #210

Merged
merged 5 commits into from Apr 16, 2019
Merged

ENH: Metadata-compatible manifest formats #210

merged 5 commits into from Apr 16, 2019

Conversation

thermokarst
Copy link
Contributor

@thermokarst thermokarst commented Apr 2, 2019 •
edited
Loading

Fixes #208

TODO

  • Come up with sane names for these new formats
  • Come up with sane names for the column names in the metadata
  • Transformers
  • Unit tests

@thermokarst thermokarst self-assigned this Apr 2, 2019
@thermokarst thermokarst marked this pull request as ready for review April 3, 2019 23:34
@thermokarst
Copy link
Contributor Author

I could use a hand with the two remaining TODO items, so if anything comes to mind while reviewing, @gregcaporaso, please share!

@gregcaporaso
Copy link
Member

Thanks @thermokarst! I'll review this tomorrow.

@thermokarst thermokarst added this to In Review in 2019.4 via automation Apr 9, 2019
gregcaporaso
gregcaporaso requested changes Apr 9, 2019
View reviewed changes
Copy link
Member

@gregcaporaso gregcaporaso left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @thermokarst! Just a few minor comments.

q2_types/per_sample_sequences/_format.py Outdated Show resolved Hide resolved
q2_types/per_sample_sequences/tests/test_format.py Outdated Show resolved Hide resolved
q2_types/per_sample_sequences/tests/data/absolute_manifests_v2/single-MANIFEST Outdated Show resolved Hide resolved
q2_types/per_sample_sequences/tests/test_format.py Show resolved Hide resolved
q2_types/per_sample_sequences/tests/test_transformer.py Outdated Show resolved Hide resolved
q2_types/per_sample_sequences/tests/test_transformer.py Outdated Show resolved Hide resolved
q2_types/per_sample_sequences/__init__.py Show resolved Hide resolved
@thermokarst thermokarst moved this from In Review to In Development in 2019.4 Apr 11, 2019
@thermokarst thermokarst mentioned this pull request Apr 15, 2019
Merged
@thermokarst thermokarst moved this from In Development to In Review in 2019.4 Apr 15, 2019
gregcaporaso
gregcaporaso approved these changes Apr 16, 2019
View reviewed changes
Copy link
Member

@gregcaporaso gregcaporaso left a comment

Choose a reason for hiding this comment

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

Looks good @thermokarst, no additional comments!

@gregcaporaso gregcaporaso merged commit 6942cec into qiime2:master Apr 16, 2019
2019.4 automation moved this from In Review to Completed Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregcaporaso gregcaporaso gregcaporaso approved these changes

Assignees

@gregcaporaso gregcaporaso

Labels
None yet
Projects
No open projects
2019.4
  
Completed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Metadata-compatible Manifest formats
2 participants
@thermokarst @gregcaporaso