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

Fix a minor bug in superduper typer #521

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Fix a minor bug in superduper typer #521

merged 1 commit into from
Jul 25, 2023

Conversation

thejumpman2323
Copy link
Contributor

@thejumpman2323 thejumpman2323 commented Jul 24, 2023

Description

resolves #520

Related Issue(s)

Checklist

  • Change or addition is covered by unit and/or integration tests. If bugfix, there should be at least one test that fails pre-PR and passes after.
  • Classes and functions substantially affected by this PR have sphinx format docstrings added or updated.
  • If your changes introduce modifications to functionality, behavior, or usage of the project, please ensure that the documentation is updated accordingly.

Additional Notes

@thejumpman2323 thejumpman2323 requested review from rec and blythed July 24, 2023 20:36
Copy link
Contributor

@rec rec left a comment

Choose a reason for hiding this comment

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

Thanks so much for doing this, particularly in writing the test.

There's one more error I'd like you to correct while you are here - in my code. :-D

return sum(hasattr(item, a) for a in cls.attrs) == count should be
return sum(hasattr(item, a) for a in cls.attrs) >= count.

It hasn't yet made a difference, but it's logically correct, so...

def test_mongodb_typer(self, test_db):
assert MongoDbTyper.accept(test_db.db) is True

def test_sklearn_typer(
Copy link
Contributor

Choose a reason for hiding this comment

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

One line pls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment?

from superduperdb.misc.superduper import MongoDbTyper, SklearnTyper, TorchTyper


class TestSuperDuper:
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for this class at all! That's from when we used to use unittest, back in the old, old, old days.

Make these all static functions, get rid of self, outdent them, pytest will do the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure :) thanks

Copy link
Contributor

@rec rec left a comment

Choose a reason for hiding this comment

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

Good stuff!

superduperdb/misc/superduper.py Show resolved Hide resolved
@thejumpman2323 thejumpman2323 merged commit 2421c92 into superduper-io:main Jul 25, 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.

[BUG]: superduper(db) not working for mongodb database.
3 participants