[ENH] Implements sklearn style pretty printing directly in skbase
#150
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| with suppress(AttributeError): | ||
| return base_object._sk_visual_block_() | ||
|
|
||
| if isinstance(base_object, str): |
Check warning
Code scanning / CodeQL
Unreachable code
| super().__init__(indent, width, depth, stream, compact=compact) | ||
| self._indent_at_name = indent_at_name | ||
| if self._indent_at_name: | ||
| self._indent_per_level = 1 # ignore indent param |
Check warning
Code scanning / CodeQL
Overwriting attribute in super-class or sub-class
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #150 +/- ##
==========================================
+ Coverage 82.68% 84.07% +1.39%
==========================================
Files 32 41 +9
Lines 2327 2902 +575
==========================================
+ Hits 1924 2440 +516
- Misses 403 462 +59
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@fkiraly I want to do a bit more testing and I need to clean up the 2 code scanning items that were found, but this should be ready for you to take a look at. |
|
@fkiraly this could in theory be implemented as a local configuration. But in practice, how the objects are printed or represented in html is really something you want to set and have apply to all the objects. |
|
no, I meant architecturally. Why does this PR need to depend on the global config? In my (perhaps too naive) understanding of the global config, it should not be coupled with the pretty printing config option, so it should not be necessary to have this PR depend on the global config. Conversely, if there is a strict dependency here, I think there's something wrong architecturally (perhaps subtly) |
|
@fkiraly i think you are suggesting the local config is the main/prevailing config and the global config can optionally override it. That would imply returning local config and then updating with global config. However, I think the global config should be the higher precedent config and the local config should be to optionally override it. The global configuration should only contain config params and values that provide broad context to a BaseObject. Local config can override that context for an object. So for something like pretty printing options. The most typical use case is that they are not changed from global config settings. So in a limited number of cases a user calls the object's set_config to override global setting. |
|
@RNKuhns, I am not suggesting anything about precendence of configs here (and I agree with you that local should override global). All I'm saying that the existene of a global config mechanism should not be architecturally required for the pretty printing. If it is, something is architecturally wrong. Why: config precedence and "getting the values for the applicable config" are logically separate, hence should be architecturally separate to translating the applicable config values into behaviour. |
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 <rnkuhns@gmail.com>
Reference Issues/PRs
Fixes #17
What does this implement/fix? Explain your changes.
This implements
sklearnpretty printing directly inskbasewith some minor tweaks.Does your contribution introduce a new dependency? If yes, which one?
No new dependencies. In theory this will let us remove the dependency on
sklearn, but we'll save that for a separate PR.What should a reviewer concentrate their feedback on?
This depends on the global config PR -- specifically, the ability for local config interface to retrieve (and update) global config. Two global (or local) configuration parameters (print_changed_only and display) control pretty printing output.
Any other comments?
PR checklist
For all contributions
the PR topic is related to enhancement, CI/CD, maintenance, documentation, or a bug.
For code contributions