riga / law

Build large-scale task workflows: luigi + job submission + remote targets + environment sandboxing using Docker/Singularity
http://law.readthedocs.io
BSD 3-Clause "New" or "Revised" License
98 stars 41 forks source link

Remove unused parameter job_ids #110

Closed meliache closed 3 years ago

meliache commented 3 years ago

Hi,

I wanted to look at LAW's htcondor implementation, so I cloned it and open the htcondor/job.py in emacs and pylint gave me a bunch of warnings. The one that I fixed here was that the parameter job_ids was used at no point in the parse_long_output method, 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 else after if...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

riga commented 3 years ago

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 👍