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

Autogen field names in projection package #941

Closed
fnothaft opened this Issue Feb 13, 2016 · 3 comments

Comments

Projects
2 participants
@fnothaft
Member

fnothaft commented Feb 13, 2016

@jpdna has identified a bug whose root cause has to do with a default field being used when a field is projected out using Parquet's projection feature and our org.bdgenomics.adam.projection package. The TL;DR on the bug (@jpdna can you open an issue for this and link the issue to this issue?) is that the firstOfPair/secondOfPair fields changed to readInFragment but the AlignmentRecordField enum didn't get changed, so the projection didn't get fixed. To not have this problem come up in the future, we should autogenerate the enums from the avro schemas from bdg-formats.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Sep 6, 2016

Member

Adding unit tests for projection field classes in #1144. I was hoping these could be a template for any auto-generated code, but unfortunately the tests don't fail unless the manually written assertions fail. There apparently isn't any runtime checking by Parquet.

Member

heuermh commented Sep 6, 2016

Adding unit tests for projection field classes in #1144. I was hoping these could be a template for any auto-generated code, but unfortunately the tests don't fail unless the manually written assertions fail. There apparently isn't any runtime checking by Parquet.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Mar 3, 2017

Member

This can be resolved using the codegen from #1391.

Member

fnothaft commented Mar 3, 2017

This can be resolved using the codegen from #1391.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft May 12, 2017

Member

I will take this once #1391 merges.

Member

fnothaft commented May 12, 2017

I will take this once #1391 merges.

fnothaft added a commit to fnothaft/adam that referenced this issue May 24, 2017

fnothaft added a commit to fnothaft/adam that referenced this issue May 24, 2017

fnothaft added a commit to fnothaft/adam that referenced this issue Jun 22, 2017

fnothaft added a commit to fnothaft/adam that referenced this issue Jun 24, 2017

fnothaft added a commit to fnothaft/adam that referenced this issue Jun 26, 2017

fnothaft added a commit to fnothaft/adam that referenced this issue Jul 11, 2017

fnothaft added a commit to fnothaft/adam that referenced this issue Jul 11, 2017

@heuermh heuermh moved this from Triage to Completed in Release 0.23.0 Jan 4, 2018

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