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

Cleanup python deephaven package dependencies #2538

Merged

Conversation

devinrsmith
Copy link
Member

@devinrsmith devinrsmith commented Jun 14, 2022

I don't think we have a real dependency on dill nor wrapt. Since we only support >= 3.7, numba is always required. And enum34 is never required. The Deprecated classes should eventually be removed, nothing uses them except tests.

jmao-denver
jmao-denver previously approved these changes Jun 14, 2022
Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

LGTM

@devinrsmith
Copy link
Member Author

Potentially we should just delete WorkerPythonEnvironment, PickledResult, and python-engine-test. None of it is run, see #734

@devinrsmith
Copy link
Member Author

I've gone ahead with the further cleanup here.

@devinrsmith devinrsmith force-pushed the cleanup-deephaven-python-dependencies branch from c0b8b22 to ad46a93 Compare June 24, 2022 19:53
@devinrsmith
Copy link
Member Author

I've removed the file deletions, per @niloc132:

moving it to the python-engine-test stuff seems cheaper long term, since at least we want to get something like those tests running at some point, rather than deleting and restoring imho

Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

LGTM

@devinrsmith devinrsmith dismissed chipkent’s stale review June 27, 2022 18:18

Not removing these files anymore.

@devinrsmith devinrsmith merged commit ce8c999 into deephaven:main Jun 27, 2022
@devinrsmith devinrsmith deleted the cleanup-deephaven-python-dependencies branch June 27, 2022 18:18
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants