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

made changes to lsf.py to make it compatible on lsf9 and lsf10 with p… #2726

Merged
merged 4 commits into from
Jul 4, 2019
Merged

Conversation

drkennetz
Copy link
Collaborator

…ython3x

@drkennetz
Copy link
Collaborator Author

@drjrm3
@mr-c

src/toil/batchSystems/lsf.py Outdated Show resolved Hide resolved
src/toil/batchSystems/lsf.py Outdated Show resolved Hide resolved
@drkennetz
Copy link
Collaborator Author

@adamnovak When you get a chance could you take a look. This is to incorporate python3.x compatibility for lsf9/10 integration.

Copy link
Member

@adamnovak adamnovak left a comment

Choose a reason for hiding this comment

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

I have some quibbles with the character encoding, and saw a couple things that I think need more justification. But once that's addressed this looks OK.

src/toil/batchSystems/lsf.py Outdated Show resolved Hide resolved
src/toil/batchSystems/lsf.py Show resolved Hide resolved
src/toil/batchSystems/lsf.py Outdated Show resolved Hide resolved
changed all character encodings to utf-8 to make encoding consistent across code base.
@drkennetz
Copy link
Collaborator Author

@adamnovak pushed changes for character encodings.

src/toil/batchSystems/lsf.py Outdated Show resolved Hide resolved
@adamnovak
Copy link
Member

This PR will fix #2724

@drkennetz
Copy link
Collaborator Author

@adamnovak any status update on this?

@adamnovak
Copy link
Member

OK, I've imported this in for full testing. When that passes we can go ahead and merge it.

@mr-c mr-c merged commit dc28240 into DataBiosphere:master Jul 4, 2019
@mr-c
Copy link
Contributor

mr-c commented Jul 4, 2019

Thank you @drkennetz !

@drkennetz
Copy link
Collaborator Author

Thanks for your help all! @mr-c @adamnovak @drjrm3

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.

None yet

4 participants