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

[ENH] Separate Pretty Printing from sklearn dependency #17

Closed
RNKuhns opened this issue Jul 1, 2022 · 0 comments · Fixed by #156
Closed

[ENH] Separate Pretty Printing from sklearn dependency #17

RNKuhns opened this issue Jul 1, 2022 · 0 comments · Fixed by #156
Labels
API design API design & software architecture Interface compatability Ensure compatibility of interface with related packages
Milestone

Comments

@RNKuhns
Copy link
Contributor

RNKuhns commented Jul 1, 2022

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is.

Eventually the baseobject package will not depend on scikit-learn's BaseEstimator. This will be accomplished by gradually removing the BaseObject dependencies on BaseEstimator for different API interfaces and features.

This issue covers the features that need to be added to BaseObject to remove the dependency on BaseEstimator when pretty printing instances of classes that inherit from BaseObject.

Describe the solution you'd like
A clear and concise description of what you want to happen, ideally taking into consideration the existing toolbox design, classes and methods.

BaseObject should contain all the functionality needed to implement the pretty printing. However, we need to investigate and eventually decide the degree to which the functionality in BaseObject should identically recreate the pretty printing functionality present in BaseEstimator.

Important considerations are:

  • Ease of maintence
    • There may be ways we can get a similar effect with less code to maintain, but we need to understand how the pros and cons associated with any different implementations of this functionality
  • How BaseObject interacts with sklearn classes/pipelines when pretty printing representations of classes
  • Need for adequate tests to ensure pretty printing is working correctly and meets the level of compatability with sklearn that we choose
@RNKuhns RNKuhns added this to the Release 0.2.0 milestone Jul 1, 2022
@RNKuhns RNKuhns added the API design API design & software architecture label Jul 1, 2022
@RNKuhns RNKuhns modified the milestones: Release 0.2.0, Release 0.4.0 Oct 6, 2022
@RNKuhns RNKuhns modified the milestones: Release 0.4.0, Release 0.5.0 Jan 8, 2023
@RNKuhns RNKuhns changed the title Separate Pretty Printing from sklearn dependency [ENH] Separate Pretty Printing from sklearn dependency Jan 8, 2023
@RNKuhns RNKuhns added the Interface compatability Ensure compatibility of interface with related packages label Jan 9, 2023
fkiraly added a commit that referenced this issue Apr 18, 2023
Fixes #17.

This implements ``sklearn`` pretty printing directly in ``skbase`` with
some minor tweaks.

Based on #150 by @RNKuhns, but
removes the strong coupling to the new global config interface, as well
as the global config interface that imo has nothing to do (or should
have nothing to do) with that PR.

I think the global config interface still needs some work, and according
to my comments in #150, it should be entirely uncoupled, or more
precisely, coupled only via `get_config`.

(the alternative would be coupling it to every place where a config -
local or global - is used, which is bad design)

---------

Co-authored-by: rnkuhns <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API design API design & software architecture Interface compatability Ensure compatibility of interface with related packages
Projects
None yet
1 participant