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

Slightly confusing behaviour for default nb_workers argument handling in initialize #177

Open
GaneshParkopedia opened this issue Apr 6, 2022 · 3 comments · May be fixed by #268
Open

Slightly confusing behaviour for default nb_workers argument handling in initialize #177

GaneshParkopedia opened this issue Apr 6, 2022 · 3 comments · May be fixed by #268

Comments

@GaneshParkopedia
Copy link

Thanks for your work on this project, it has been working great for us.
There's a small thing about the handling of the nb_workers argument of pandarallel.initialize I'd like to suggest.

Currently the default value is generated at the top of the file as a global, then set as the default argument of the function. This means if my program has an n_cpus-type Optional argument, I can't just set this as the nb_workers argument as this would overwrite the default value and break things.

Ideally it would be

class pandarallel:
    @classmethod
    def initialize(
        cls,
        shm_size_mb=None,
        nb_workers=None,
        progress_bar=False,
        verbose=2,
        use_memory_fs: Optional[bool] = None,
    ) -> None:

    nb_workers = (
            nb_workers if nb_workers is not None else NB_PHYSICAL_CORES
    )

This would also match the argument handling of use_memory_fs.

I can raise a PR for this if there's interest

@till-m
Copy link
Collaborator

till-m commented Aug 22, 2022

Hi @GaneshParkopedia,

thanks for your interest in this package and your willingness to contribute. Sorry for the delay.
I don't think I am following your intended behaviour exactly. Could you describe again what you'd like to do that doesn't work using the current setup?

Irrespective of that, I think it there's an argument to be made that the behaviour of nb_workers and use_memory_fs should be the same.

@nalepae
Copy link
Owner

nalepae commented Jan 23, 2024

Pandaral·lel is looking for a maintainer!
If you are interested, please open an GitHub issue.

@shermansiu
Copy link

Wrote a PR: this is a relatively easy fix.

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 a pull request may close this issue.

4 participants