Closed Mithil467 closed 4 months ago
Attention: 3 lines
in your changes are missing coverage. Please review.
Comparison is base (
4666931
) 86.72% compared to head (7c98541
) 91.28%.
Files | Patch % | Lines |
---|---|---|
pandarallel/progress_bars.py | 50.00% | 3 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi @Mithil467,
thanks for the PR. Could you please add a test for this case?
Sure. I noticed that when progress_bar is True, we get ZeroDivisionError. In order to fix that, I would need to know what we expect UI wise. I personally like [2] more than [1] as it gives a sense of success, but would like to know your opinions or if you want to do things differently. Hence, I have added [2] for now, let me know if it needs changes.
Consoles:
Notebook:
Personally, I prefer option 1.: The grey bar is neither failure (red) nor success (green), similarly, the processing of the empty DataFrame didn't really succeed or fail. @nalepae do you have an opinion on the matter?
@till-m @nalepae If you say so, I can make the necessary changes to implement option [1]. Should I go ahead?
Hi dev team, any updates on this one?
My apologies, I am not maintaining this project anymore, hence me not responding. But I can make an exception and see this PR through.
I would suggest going with [1]. @Mithil467 could you kindly ping me when/if you've implemented that?
@till-m @nalepae I have implemented the progress bar change to type [1]. Please review.
Pandaral·lel is looking for a maintainer! If you are interested, please open an GitHub issue.
Thanks for the contribution :)
Thanks for the help! @till-m
Since nb_item comes out as 0 for empty dataframes and series, we were returning an empty list from the
chunk
function. Hence, we were yielding nothing from ourDataType.get_chunks
method which caused ourchunks
list being empty and nb_workers = len(chunks) = 0.Let me know if this fix seems good enough, and also if we need to add any new tests.
Fixes #115, fixes #141.