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

Added Tests to Schulze Functions in Core Package. #103

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

CheatCodeSam
Copy link

Added a few starter unit test for the Schulze functions.
I added an init file in the test folder so it would be easier to import the elekto module.
I added pytests and pandas into the requirements.txt so that they would be installed with pip install.
I gave the Schulze functions type hints so they're a little easier to work around with, and write tests around.

Let me know if this was what you were looking for, I'm happy to keep testing the other modules of this application.

@jberkus
Copy link
Member

jberkus commented Jan 25, 2025

Why Pandas? Asking because that imports a LOT of modules.

@jberkus
Copy link
Member

jberkus commented Jan 25, 2025

Otherwise, this looks great, lemme try it out.

@CheatCodeSam
Copy link
Author

CheatCodeSam commented Jan 25, 2025

It's was already being used within test/test_main.py, and it looks like elekto.core.Election.from_csv expects a Dataframe object. I just went ahead and made sure if it was being installed with the rest of the requirements.txt.

In all honesty it probably wouldn't be too difficult to remove the need for the dependency entirely. I'm pretty sure the only thing that needs it is elekto.core.Election.from_csv.

@jberkus
Copy link
Member

jberkus commented Jan 29, 2025

@CheatCodeSam sorry for delay here, as expected your adding these tests exposed some unrelated problems that prevent testing the PR, specifically;

  1. we need a way to run tests that don't require configuring a database, probably by loading SQLite.
  2. also, the "no database found" Exception itself has a bug

I'll work on these starting with (2), of course your ideas welcome.

@CheatCodeSam
Copy link
Author

Awesome, thank you for getting back to me on that. I've been poking around a few of the internals some more and I might have found some stuff that we could leverage for that first one. I'll append it to the PR and keep you in the loop.

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.

2 participants