Skip to content

Is the internal twosigma refactor of window.pyx planned for merging in pandas? #27446

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

Closed
ghost opened this issue Jul 18, 2019 · 4 comments
Closed

Comments

@ghost
Copy link

ghost commented Jul 18, 2019

I've been thinking of improvements to pandas rolling window functionality. Namely, allowing users to supply their own indexers and allowing numba JIT'd kernels to be passed to apply/transform.

I floated the idea of a window.pyx refactor, and was told that "others are working on it". I've seen no issue for this on pandas-dev/pandas, but there's apperently an effort going privately at Twosigma to do a major redesign of this part of pandas.

Of course they're free to develop whatever they want internally, but if this is meant for inclusion in pandas, why is the design not being discussed here? and if it's an internal effort only (perfectly fine), why should their efforts influence efforts by contributors such as myself?

I'd kindly to like to have this clarified before I waste time working on my own PR.
There seems to be an intention to push some bits to pandas, but not others ?

jreback: I would do this versus pandas/master. Best to push whatever we can there
in small uncontroversial bits.

See #27305, and twosigma#4

cc @jreback , @mroeschke

@ghost ghost changed the title Is the internal twosigma refactor of window.pyx meant for mergin in pandas? Is the internal twosigma refactor of window.pyx planned for merging in pandas? Jul 18, 2019
@mroeschke
Copy link
Member

mroeschke commented Jul 18, 2019

The windowing refactor in that repo is thus far only specific to twosigma. We would like to contribute the changes back to the main pandas repo; however, there will definitely be a discussion first with the other core developers/community around a POC before any further action.

The PRs you have cross referenced, as @jreback mentioned, are beneficial for the main pandas project (test cleanups and typing).

@jreback jreback closed this as completed Jul 18, 2019
@ghost
Copy link
Author

ghost commented Jul 18, 2019

Thanks for clarifying. In the meantime, do you plan to block other enhancements to the code which conflict with your plans? it can be problematic transperancy-wise to have PRs included or rejected by folks conducting their own competing private effort.

If you're going ahead with development without a discussion with the community or other core developers, and without offering a timeline (and of course you're not obligated to), What happens when someone submits a PR that conflicts with your undisclosed plans? will you reject PRs indefinitely simply because you have a private plan to perhaps eventually, propose your private code upstream? you can see the potential for conflicts of interest there I'm sure.

@mroeschke
Copy link
Member

I understand the concern. Any enhancement proposal to the pandas repo will not be out-right blocked due to a competing private effort since pandas is open source first and foremost.

At the end of the day, the changes in the twosigma repo will be a proposal to the main pandas library and may not be suitable for merging even if it was the only enhancement proposal for windowing. Not sure who mentioned "others are working on it", but I imagine the sentiment is awareness about another effort as opposed to preventing your proposal.

@ghost
Copy link
Author

ghost commented Jul 19, 2019

In that case, I plan to submit these changes soon and I hope that there will indeed be no difficulty beyond the standard code review thumb-wrestling.

And thank you for the direct and candid response, I appreciate it.

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

No branches or pull requests

2 participants