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

Fix ValueError for Empty DataFrames: Ensure Process Count is at Least 1 #245

Merged
merged 5 commits into from
Feb 16, 2024

Conversation

Mithil467
Copy link
Contributor

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 our DataType.get_chunks method which caused our chunks 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.

@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

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 ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
+ Coverage   86.72%   91.28%   +4.55%     
==========================================
  Files          12       12              
  Lines         580      585       +5     
==========================================
+ Hits          503      534      +31     
+ Misses         77       51      -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@till-m
Copy link
Collaborator

till-m commented Jun 26, 2023

Hi @Mithil467,

thanks for the PR. Could you please add a test for this case?

@Mithil467
Copy link
Contributor Author

Mithil467 commented Jun 26, 2023

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:

  1. image
  2. image

Notebook:

  1. image
  2. image

@till-m
Copy link
Collaborator

till-m commented Jun 27, 2023

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?

@Mithil467
Copy link
Contributor Author

@till-m @nalepae If you say so, I can make the necessary changes to implement option [1]. Should I go ahead?

@SiRumCz
Copy link

SiRumCz commented Jan 22, 2024

Hi dev team, any updates on this one?

@till-m
Copy link
Collaborator

till-m commented Jan 23, 2024

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?

@Mithil467
Copy link
Contributor Author

@till-m @nalepae I have implemented the progress bar change to type [1]. Please review.

@nalepae
Copy link
Owner

nalepae commented Feb 4, 2024

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

Copy link
Collaborator

@till-m till-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - checked that it works in a small script and in a notebook on my machine. Thanks for the PR + also the tests! Will merge soon.

@till-m till-m merged commit 261a652 into nalepae:master Feb 16, 2024
64 of 65 checks passed
@till-m
Copy link
Collaborator

till-m commented Feb 16, 2024

Thanks for the contribution :)

@Mithil467
Copy link
Contributor Author

Thanks for the help! @till-m

@Mithil467 Mithil467 deleted the empty-df-fix branch February 16, 2024 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants