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

Handle --per-contig grouping with no gene tags #577

Merged
merged 5 commits into from
Feb 28, 2023

Conversation

akmorrow13
Copy link
Contributor

Replaces #570, adds unit test

@akmorrow13
Copy link
Contributor Author

Can one of the admins verify this patch?

@akmorrow13
Copy link
Contributor Author

@TomSmithCGAT can you trigger these tests?

@TomSmithCGAT
Copy link
Member

TomSmithCGAT commented Feb 27, 2023

@akmorrow13 - Thanks for the prompt

Please ignore the error. That's because we are using nose, which is incompatible with python 3.10 (nose-devs/nose#1099 & #546) and we appear to have added py 3.10 to our testing when removing py 3.6 (8a7c9c4).

I'll fix the tests on master and then re-run here.

@akmorrow13
Copy link
Contributor Author

@TomSmithCGAT thank you for your help. There was a bug in my test, it should now be fixed.

@TomSmithCGAT
Copy link
Member

OK, this is very strange. With the above further commits, I can get the new test to pass with py3.7 & 3.8 but not 3.9 😖

I've can replicate this behaviour locally by switching python versions, so it does seem to be something inherently different between the python versions. I understand dictionaries output order changed recently, but that should all be python>=3.6.

Leave this with me and I'll try and work out what's going on.

@TomSmithCGAT
Copy link
Member

🎉 Test passing

@IanSudbery - Whatever the difference is between py3.8 & 3.9, it manifests as differences in the output order. No reason for our tests to fail for different order. Previously, we had a 'Sort' flag for tests where the stdout should be sorted prior to comparison with reference. I've extended this so that it sorts all reference and output files prior to comparison. We really should switch to a more standard test framework when one of has some spare time!

Are you happy for me to merge this?

@akmorrow13
Copy link
Contributor Author

Thank you so much for all the work on this @TomSmithCGAT!

@IanSudbery
Copy link
Member

Looks good, go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants