Skip to content

[ENH] Add util tests#125

Closed
RNKuhns wants to merge 8 commits into
sktime:mainfrom
RNKuhns:add_util_tests
Closed

[ENH] Add util tests#125
RNKuhns wants to merge 8 commits into
sktime:mainfrom
RNKuhns:add_util_tests

Conversation

@RNKuhns
Copy link
Copy Markdown
Contributor

@RNKuhns RNKuhns commented Feb 5, 2023

Reference Issues/PRs

Completes the portion of #49 and #19 for the skbase.utils submodule. Note that I've included docstring examples for the functions even if they aren't public because I think it is helpful for developers to be able to get a quick glimpse of what the function does.

What does this implement/fix? Explain your changes.

The changes reflect:

  • Adding basic tests of skbase.utils (functions are tested further in tests of their downstream uses)
  • Improved docstrings of skbase.utils functionality (I believe these all now comply with numpydoc standard and have examples)
  • Renamed functions to be non-public and updated the names in import statements and calls elsewhere in the package
  • made minor improvements to functionality based on things I noticed when creating the tests/documenting the functionality that would easily make it more robust or improve its generality.

Does your contribution introduce a new dependency? If yes, which one?

No new dependencies.

What should the reviewer focus their review on

I believe I added reasonable tests of the basic functionality of these utils (they are implicitly further tested in their downstream uses). So the key thing is whether you agree with the renaming to make the functionality non-public (i.e., not part of public API). Let me know if any of the functions in skbase.utils._iter.py or skbase.utils._nested_iter.py seem like something we want to include in the public API. Happy to switch the naming convention and add them to the API Reference.

PR checklist

For all contributions
  • I've reviewed the project documentation on contributing
  • I've added myself to the list of contributors.
  • The PR title starts with either [ENH], [CI/CD], [MNT], [DOC], or [BUG] indicating whether
    the PR topic is related to enhancement, CI/CD, maintenance, documentation, or a bug.
For code contributions
  • Unit tests have been added covering code functionality
  • Appropriate docstrings have been added (see documentation standards)
  • New public functionality has been added to the API Reference

@RNKuhns RNKuhns changed the title Add util tests [ENH] Add util tests Feb 5, 2023
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 5, 2023

Codecov Report

❌ Patch coverage is 69.13580% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.66%. Comparing base (7445d80) to head (624826f).
⚠️ Report is 396 commits behind head on main.

Files with missing lines Patch % Lines
skbase/base/_meta.py 30.55% 25 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
- Coverage   81.93%   81.66%   -0.27%     
==========================================
  Files          30       31       +1     
  Lines        2236     2280      +44     
==========================================
+ Hits         1832     1862      +30     
- Misses        404      418      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread skbase/tests/test_meta.py
from skbase.base._meta import BaseMetaObject


class SomeClass(BaseObject):

Check warning

Code scanning / CodeQL

`__eq__` not overridden when adding attributes

The class 'SomeClass' does not override ['__eq__'](1), but adds the new attribute [z](2).
Comment thread skbase/tests/test_meta.py
self.z = z


class MetaObjectTester(BaseMetaObject, BaseObject):

Check warning

Code scanning / CodeQL

`__eq__` not overridden when adding attributes

The class 'MetaObjectTester' does not override ['__eq__'](1), but adds the new attribute [a](2). The class 'MetaObjectTester' does not override ['__eq__'](1), but adds the new attribute [b](3). The class 'MetaObjectTester' does not override ['__eq__'](1), but adds the new attribute [c](4). The class 'MetaObjectTester' does not override ['__eq__'](1), but adds the new attribute [some_step_attr](5).
@RNKuhns
Copy link
Copy Markdown
Contributor Author

RNKuhns commented Feb 5, 2023

Didn't realize I had branched from the branch where I was working on changes to skbase.base._meta.py instead of main. Closing in favor of #126.

@RNKuhns RNKuhns closed this Feb 5, 2023
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.

3 participants