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

Add PyPy 3.9 to the testing #2

Closed
wants to merge 9 commits into from
Closed

Conversation

cclauss
Copy link
Collaborator

@cclauss cclauss commented Jan 3, 2024

Both currently supported versions of PyPy3 fail our tests. :-( https://www.pypy.org/download.html

@ddoroshev Do you have any ideas about how to fix?

@ddoroshev
Copy link

Do you have any ideas about how to fix?

Well, I have zero ideas about what going on there. Actually, I know just a little bit about PyPy, and I know nothing about whoosh library and its tests. But, looking at the failing test, I can tell that a few hours of step-by-step debugging, comparing the behaviour between CPython and PyPy, will give you the answers.

@ZeroCool940711
Copy link
Contributor

ZeroCool940711 commented Jan 3, 2024

I'm looking into this, I think I have sort of found where one of the issues is tho. I think we have to look more closely at the tests we have. I fixed an issue with tests/test_automata.py but I'm now getting errors in other tests which I am looking into.

ZeroCool940711 added a commit that referenced this pull request Jan 3, 2024
Fix GitHub Actions by Installing 'jieba' Module
@cclauss
Copy link
Collaborator Author

cclauss commented Jan 4, 2024

Even the latest version of PyPy (v7.3.14) makes no difference,

Add DFA.__repr__() to aid debugging...

    assert dfa == good
AssertionError: assert DFA:   2\n  3\n@ 5\n == DFA: @ 1\n  2\n  3\n

@ZeroCool940711
Copy link
Contributor

Even the latest version of PyPy (v7.3.14) makes no difference,

Add DFA.__repr__() to aid debugging...

    assert dfa == good
AssertionError: assert DFA:   2\n  3\n@ 5\n == DFA: @ 1\n  2\n  3\n

Yeah, I tried debugging the test_minimize_dfa function and a few other parts of the code with no luck. Maybe using an older version of PyPy might help, it could be that something changed on the newer versions, remember that Whoosh hasn't been updated in more than 5 years, maybe we should look into what version was used back then and if it works now, then start updating from there.

@cclauss
Copy link
Collaborator Author

cclauss commented Jan 5, 2024

https://downloads.python.org/pypy/ pypy-3.8-v7.3.6 is the earliest version that supports Py38 and it still fails in the same way.

@ZeroCool940711
Copy link
Contributor

@cclauss I found a library called Pythomata which I think we can use to replace most if not all the code from the src/whoosh/automata folder and some parts of the test files we use that are giving us trouble. We could either just add the dependency to the setup.py to use the library directly or we could just copy/paste the code and reuse it as part of Whoosh itself. I think it's better to just use the library itself since it will reduce the size of the source code and make it easier to maintain and if the Pythomata library is updated in the future we can benefit from the changes, if things break we can just use an older version of the library that works until things get fixed on our end.

@cclauss
Copy link
Collaborator Author

cclauss commented Feb 3, 2024

Solarlint does not support Python. Use ruff instead. https://docs.astral.sh/ruff

@ZeroCool940711
Copy link
Contributor

Solarlint does not support Python. Use ruff instead. docs.astral.sh/ruff

I was testing it because it looks interesting. Anything that can help to improve our code is welcome. Their bot and website seem to do a pretty good job with Python tho, not sure about their VSCode extension tho. I will check ruff as well and see how they compare.

Copy link

codecov bot commented Feb 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (059137a) 80.97% compared to head (6557c5a) 80.97%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #2   +/-   ##
=======================================
  Coverage   80.97%   80.97%           
=======================================
  Files         133      133           
  Lines       29496    29496           
  Branches     5066     5066           
=======================================
  Hits        23884    23884           
  Misses       4737     4737           
  Partials      875      875           

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

@ZeroCool940711 ZeroCool940711 marked this pull request as draft February 9, 2024 11:13
Copy link

sonarqubecloud bot commented Feb 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
2.7% Duplication on New Code

See analysis details on SonarCloud

@cclauss cclauss deleted the patch-2 branch February 19, 2024 10:21
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