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

missing self arguments #6188

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

fchapoton
Copy link
Contributor

add some missing self arguments in keywords, as suggested by mypy

@roed314
Copy link
Contributor

roed314 commented Sep 12, 2024

I agree that there's something to be fixed here. I think having def runTest(self): pass disables scanning for tests. I'm not sure why we want to do that; should we instead delete all these functions and use the default runTest from unittest.TestCase?

@fchapoton
Copy link
Contributor Author

fchapoton commented Sep 12, 2024

I have no idea. There are only a few definitions of runTest :


lmfdb/classical_modular_forms/cmf_test_pages.py:    def runTest(self):
lmfdb/classical_modular_forms/test_cmf.py:    def runTest(self):
lmfdb/classical_modular_forms/test_cmf2.py:    def runTest(self):
lmfdb/ecnf/ecnf_test_pages.py:    def runTest(self):
lmfdb/genus2_curves/g2c_test_pages.py:    def runTest(self):
lmfdb/groups/abstract/groups_test_pages.py:    def runTest(self):
lmfdb/siegel_modular_forms/smf_test_pages.py:    def runTest(self):

@fchapoton
Copy link
Contributor Author

should I change the PR to remove all these methods ?

@roed314
Copy link
Contributor

roed314 commented Sep 13, 2024

That sounds good. We can see what happens with the tests.

@edgarcosta
Copy link
Member

I should have said that these tests are not run by default, as these are the tests that test all webpages of some kind

@fchapoton
Copy link
Contributor Author

so should I step back to just adding the missing self ?

@fchapoton
Copy link
Contributor Author

Voilà

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