Remove unused parameter job_ids #110
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @meliache , thanks for opening the PR! Yep, I guess this change can be merged right away. Updates to the code style are actually on my list :) I'm planning to release 1.0 later this year, which mainly requires the full documentation and unit tests. I'm going to tackle code style after the latter are implemented, just to be sure everything stays stable. In fact, I stumbled upon b2luigi already quite a while ago (ErUM / conferences) and noticed the similarities. I'll reach out to you next week via a different channel if that's okay for you. Maybe we can have a chat on these topic 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
I wanted to look at LAW's htcondor implementation, so I cloned it and open the
htcondor/job.pyin emacs and pylint gave me a bunch of warnings. The one that I fixed here was that the parameterjob_idswas used at no point in theparse_long_outputmethod, so I removed it to avoid confusion.The other warnings were mostly the no-else-return warning by pylint because of the unnecessary use of
elseafterif...return, but I think that is a question of style, e.g. kubernetes also use this under the name space shuttle style.By the way, unrelated to the PR, I was looking at LAW's htcondor implementation because I was curious how it compares to the htcondor implementation in b2luigi, which is Luigi extension that had been developed by my former colleague @nils-braun at Belle II and that I started using before I even knew of LAW (and which I'm now maintaining after Nils left the collaboration). I was asked by other members whether it wouldn't make sense to try to avoid duplicating efforts like e.g. implementing different batch systems, though I'm not sure how easy that would be, since the designs of the respective packages differ quite a bit and supporting a batch system isn't actually that difficult as long as it has a well-designed interface. Sadly I'm at the end of my PhD and currently lacking the time doing a major re-write of our package, but in general I'm always open to discussion, which however can happened outside this PR.
Cheers,
Michael Eliachevitch