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

updates for next rebuild cycle #9

Merged
merged 6 commits into from
Nov 4, 2021
Merged

updates for next rebuild cycle #9

merged 6 commits into from
Nov 4, 2021

Conversation

k-dominik
Copy link

Needed to bump some dependencies, with side effects.
Removed al unnecessary strict pins

comparisons to vigra should take into account that calculations there, including
intermediate results are done in float32 - hence the mismatch
@k-dominik k-dominik force-pushed the py38py39 branch 4 times, most recently from f89dc0d to 9fbdac1 Compare November 4, 2021 13:02
removed unnecessary pins.
this recipe has less strict pins on dependencies, allowing for a wider compatibility.
if group_index[0] < 0:
group_index[0] += 1
return None

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why it makes sense to remove these lines. According to the docs linked in the comments, GroupBy.apply() retains the same quirky behavior in pandas 1.x that it always had: it processes the first group twice. Since this function stores its results via a side-effect, we have to "skip" the first pass, right?

Copy link
Member

Choose a reason for hiding this comment

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

Admittedly, relying on side-effects is generally an awkward solution I'd usually prefer to avoid. That's exactly what is complicating this analysis. There might be a smarter way to implement this function, but I don't have time to think about it now. (When I originally wrote this code, I wasn't super well-versed in the pandas API.)

Copy link
Author

@k-dominik k-dominik Nov 4, 2021

Choose a reason for hiding this comment

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

phew, good that you had a look. I have to admit that I based the removal on the 0.25 release notes:
https://pandas.pydata.org/pandas-docs/stable/whatsnew/v0.25.0.html#groupby-apply-on-dataframe-evaluates-first-group-only-once

I experimentally verified that the behavior with pandas 1.3.4 is consistent with the fix announced in 0.25...

Copy link
Member

Choose a reason for hiding this comment

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

Ah. Okay, thanks and sorry for the noise. The pandas documentation seems to be incorrect! I tried to search through the history of this issue on the pandas issue tracker, and became hopelessly confused.

Copy link
Author

Choose a reason for hiding this comment

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

I got also confused, so I resorted to brute force:

in the table the "count" indicates how often the function was called (same example as in the 0.25 release notes). 3 means the some item is evaluated twice, 2 means that weird behavior is gone...

build count
0.23.4=py37h637b7d7_1000 3
0.23.4=py37hf8a1672_0 3
0.24.0=py37hf484d3e_0 3
0.24.1=py37hf484d3e_0 3
0.24.2=py37hb3f55d8_0 3
0.24.2=py37hb3f55d8_1 3
0.24.2=py37hf484d3e_0 3
0.25.0=py37hb3f55d8_0 2
0.25.1=py37hb3f55d8_0 2
0.25.2=py37hb3f55d8_0 2
0.25.3=py37hb3f55d8_0 2
1.0.0=py37hb3f55d8_0 2
1.0.1=py37hb3f55d8_0 2
1.0.2=py37h0da4684_0 2
1.0.3=py37h0da4684_0 2
1.0.3=py37h0da4684_1 2
1.0.4=py37h0da4684_0 2
1.0.5=py37h0da4684_0 2
1.1.0=py37h3340039_0 2
1.1.1=py37h3340039_0 2
1.1.2=py37h3340039_0 2
1.1.3=py37h3340039_0 2
1.1.3=py37h9fdb41a_1 2
1.1.3=py37h9fdb41a_2 2
1.1.3=py37hb33c840_2 2
1.1.4=py37h10a2094_0 2
1.1.5=py37hdc94413_0 2
1.2.0=py37h40f5888_1 2
1.2.0=py37hdc94413_0 2
1.2.0=py37hdc94413_1 2
1.2.1=py37h40f5888_0 2
1.2.1=py37hdc94413_0 2
1.2.2=py37h40f5888_0 2
1.2.2=py37hdc94413_0 2
1.2.3=py37h40f5888_0 2
1.2.3=py37hdc94413_0 2
1.2.4=py37h219a48f_0 2
1.2.4=py37h40f5888_0 2
1.2.5=py37h219a48f_0 2
1.2.5=py37h40f5888_0 2
1.3.0=py37h219a48f_0 2
1.3.1=py37h219a48f_0 2
1.3.1=py37h40f5888_0 2
1.3.2=py37h40f5888_0 2
1.3.2=py37he8f5f7f_0 2
1.3.3=py37h40f5888_0 2
1.3.3=py37he8f5f7f_0 2
1.3.4=py37h40f5888_0 2
1.3.4=py37he8f5f7f_0 2

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 this pull request may close these issues.

2 participants