...

(2) The patch adds new functionality to use multiple processes in parallel. The normal parameter values are integers indicating how many processes to use. The parameter also needs two special values -- one to indicate "use os.cpu_count", and the other to indicate "don't use multiprocessing at all".

(A) Is there a Best Practices for this situation, with two odd cases?

No. In this situation I would consider 0 or -1 for "use os.cpu_count' and None for "don't use multi-processing".

Why would the user care if multiprocessing is used behind the scene? It would be strange for processes=1 to fail if multiprocessing is not available. If you set a default value of 1, then compileall() will work regardless of whether multiprocessing is available. In short: processes <= 0: use os.cpu_count() processes == 1 (default): just use normal sequential compiling processes > 1: use multiprocessing There's no reason to introduce None. Or am I missing something?

On Mon, Apr 28, 2014 at 12:56 PM, Charles-François Natali wrote:

Why would the user care if multiprocessing is used behind the scene?

Err ... that was another set of questions that I forgot to ask. (A) Why bother raising an error if multiprocessing is unavailable? After all, there is a perfectly fine fallback... On other hand, errors should not pass silently. If a user has explicitly asked for multiprocessing, there should be some notice that it didn't happen. And builds are presumably something that a developer will monitor to respond to the Exception. (A1) What sort of Error? I'm inclined to raise the original ImportError, but the patch prefers a ValueError. (B) Assuming the exception, I suppose your question adds a 3rd special case of "Whatever the system suggests, and I don't care whether or not it involves multiprocessing."

It would be strange for processes=1 to fail if multiprocessing is not available.

As Claudiu pointed out, processes=1 should really mean 1 worker process, which is still different from "do everything in the main process". I'm not sure that level of control is really worth the complexity, but I'm not certain it isn't.

processes <= 0: use os.cpu_count()

I could understand doing that for 0 or -1; what is the purpose of doing it for both, let alone for -4? Are we at the point where the parameter should just take positive integers or one of a set of specified string values? -jJ

In article , "Jim J. Jewett" wrote:

As Claudiu pointed out, processes=1 should really mean 1 worker process, which is still different from "do everything in the main process". I'm not sure that level of control is really worth the complexity, but I'm not certain it isn't.

For regrtest, there is an important difference between "do everything in the main process" and "do one test at a time in a subprocess", namely the inadvertent global side-effects that running some tests have on other tests. The latter option is much safer and gives more reproducible test results. For compileall, I don't think there is a big difference between the two cases such that the regrtest semantics need to be followed. -- Ned Deily, nad@acm.org

On 4/28/2014 4:24 PM, Claudiu Popa wrote:

This issue raised too much bikeshedding. To wrap it up, I'll modify the patch with the following:

- processes renamed to workers - `workers` defaults to 1 - When `workers` is equal to 0, then `os.cpu_count` will be used - When `workers` > 1, multiple processes will be used - When `workers` == 1, run normally (no multiple processes)

I presume you mean for this to be the default.

- Negative values really should raise a ValueError (as multiprocessing.Pool and soon concurrent.futures.Thread/ProcessPoolExecutor)

Yay!

- Will raise NotImplementedError if multiprocessing can't be used (when `workers` equals to 0 or > 1)

If anyone agrees with the above, then I'll modify the patch.

Having read the thread, though not much of the issue, these look to me like the best choices for a clean, comprehensible API.

This will be its last iteration, any other bikeshedding should be addressed by the core dev who'll apply it.

-- Terry Jan Reedy

2014-04-28 21:24 GMT+01:00 Claudiu Popa :

[...]

If anyone agrees with the above, then I'll modify the patch. This will be its last iteration, any other bikeshedding should be addressed by the core dev who'll apply it.

I'm perfectly happy with those proposals.

On 28 Apr 2014 16:19, "Ezio Melotti" wrote:

On Mon, Apr 28, 2014 at 6:32 PM, Jim J. Jewett

wrote:

...

(1) Should fixes to a docstring go in with a patch, even if they aren't related to the changing functionality?

[...]

It is best if a commit changes one small thing at a time. On the other hand, Nick recently posted that the minimal overhead of a patch commit is about half an hour.

It could be much less. As an example, http://bugs.python.org/issue19943 has been closed 9 minutes after the report, it didn't have a patch and the fix had to be applied/grafted/merged on 3 branches. The time passed between the first commit and the push is less than a minute too. Clearly the time increases if you have to run the test suite or if there is a merge conflict/push race, and further decreases if there is a simple typo-fix on default only and a patch already available.

The 30 minute guesstimate was for doing it right for a patch you didn't write yourself: - final recheck that all the required pieces are there - patch import & merge commands - run the tests for every affected branch - coming up with the NEWS entry & commit message It's generally much, much faster for changes I write myself - another undesirable incentive since it further encourages us to work on our own changes over incorporating patches from others.

...

Is that overhead enough to override the one-issue-per-patch guideline?

That said, if the main fix should go only on default and it has a smaller unrelated fix that should also go on default only, then doing it together is generally OK (for some value of "unrelated" -- the fix should still be small and near the code you touched for the main fix). If the unrelated fix needs to be ported on all the branches (or in general in branches where the main fix shouldn't go), it's better to have two separate patches. If you commit the smaller fix together with the bigger one, and then decide to backport it, you will have to do a null merge and it gets slightly more complicated.

What Ezio said :) I definitely do coarser commits for CPython than I do for beaker-project.org, specifically because Gerrit provides decent tools for reviewing and merging a patch series, and we don't currently have anything like that in the CPython workflow. Cheers, Nick.

Best Regards, Ezio Melotti _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe:

https://mail.python.org/mailman/options/python-dev/ncoghlan%40gmail.com