Skip to content

Split non-ML part into a new talon-core package - #200

Open
andersk wants to merge 2 commits into
mailgun:masterfrom
andersk:talon-core-split
Open

Split non-ML part into a new talon-core package#200
andersk wants to merge 2 commits into
mailgun:masterfrom
andersk:talon-core-split

Conversation

@andersk

@andersk andersk commented Oct 3, 2019

Copy link
Copy Markdown

The nonstandard setup.py install --no-ml option didn’t work correctly with pip. We could move the ML dependencies into an extra, but that approach was previously rejected (#121): extras can’t be enabled by default, so it could have been disruptive to existing users. Instead, we split off a new talon-core package with no ML dependencies, and have talon re-export everything from it.

Fixes #130; fixes #131.

@mailgun-ci

Copy link
Copy Markdown

Can one of the admins verify this patch?

@andrewelkins

Copy link
Copy Markdown

This is fantastic. Using the fork, but LGTM.

@benbrandt

Copy link
Copy Markdown

Have this would be amazing. Any progress on merging this?

andersk added a commit to andersk/zulip that referenced this pull request Mar 12, 2021
mailgun/talon#200

Signed-off-by: Anders Kaseorg <anders@zulip.com>
timabbott pushed a commit to zulip/zulip that referenced this pull request Mar 19, 2021
mailgun/talon#200

Signed-off-by: Anders Kaseorg <anders@zulip.com>
@andersk

andersk commented May 2, 2022

Copy link
Copy Markdown
Author

@horkhe Can you please comment on whether this approach is acceptable for upstream? Zulip has been stuck on our fork of Talon for six years due to this issue and we would very much like to unfork it.

If this is acceptable, I’m happy to respin this for the current version.

andersk added 2 commits July 29, 2022 21:19
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
The nonstandard `setup.py install --no-ml` option didn’t work
correctly with pip.  We could move the ML dependencies into an extra,
but that approach was previously rejected (mailgun#121): extras can’t be
enabled by default, so it could have been disruptive to existing
users.  Instead, we split off a new talon-core package with no ML
dependencies, and have talon re-export everything from it.

Fixes mailgun#130; fixes mailgun#131.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk
andersk force-pushed the talon-core-split branch from 3ed431d to 137ea31 Compare July 30, 2022 04:35
plugyawn pushed a commit to plugyawn/armada that referenced this pull request Jul 30, 2026
mailgun/talon#200

Signed-off-by: Anders Kaseorg <anders@zulip.com>
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.

Doesn't work without ML deps; seems like a circular import issue Unable to install without the ML functionality

4 participants