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 issues and optimize tests with Tox #1465

Closed

Conversation

Mike014
Copy link

@Mike014 Mike014 commented Jan 8, 2025

Hi,

I wanted to address your previous comment about being a bot. Rest assured, I’m not a bot—I’ve simply followed your guides and used Tox to identify and resolve some issues in the project. For example:
• Fixed problems like the incorrect location of requirements.txt and inconsistencies in file references.
• Improved test coverage and addressed minor warnings across various test files.

Results:
• Python 3.11 & 3.12:
• 100% coverage achieved (17788 statements, 0 missed).
• Tests: 5139 passed, 1 xfailed, 6 skipped.
• Verified with black and isort.

Even if this PR doesn’t get merged, I genuinely appreciate the quality of this project and hope my contributions are useful. If you’d like to clarify anything or discuss further, I’m happy to join a Zoom call—I believe communication beats unanswered messages on PRs.

Best regards,
Michele Grimaldi

Screenshot 2025-01-08 at 10 30 29 Screenshot 2025-01-08 at 10 30 45

- Changed TestOpponent class to no longer use __init__ constructor
- This fixes the PytestCollectionWarning in axelrod/tests/strategies/test_player.py:353
- This modification allows the test to run without issues.

See also: Axelrod-Python#1440
…dGamePlayer class to MakesUseOfLengthAndGamePlayer in axelrod/tests/unit/test_makes_use_of.py to resolve PytestCollectionWarning.
- Fixed 'TestOpponent' import error in 'test_sequence_player.py'.
- Refactored test classes with '__init__' constructors to avoid PytestCollectionWarning.
- Addressed UserWarnings in 'test_memoryone.py' and 'test_memorytwo.py' related to default player settings.
- Handled seed reproducibility warning in 'test_player.py' to ensure deterministic test results.
- Investigated RuntimeWarning in 'test_zero_determinant.py' related to invalid division.
- Optimized test execution time by running tests in parallel using pytest-xdist.
- Addressed minor warnings related to test strategies and examples.
@drvinceknight
Copy link
Member

Sorry, we are not going to merge your PRs.

@Mike014 Mike014 deleted the fix/test-player-class-init branch January 14, 2025 15:11
@Mike014
Copy link
Author

Mike014 commented Jan 14, 2025

Hi @drvinceknight

I hope this message finds you well. I wanted to kindly ask for clarification regarding the decision not to merge my PR. My intention with this contribution was purely to dive into the project, better understand how it works, and address some issues I identified using Tox and test analysis. Specifically, I worked on resolving certain problems like:
• Correcting the placement of requirements.txt.
• Improving test coverage and addressing warnings flagged during the tests.

Regarding the mention of AI, I’d like to clarify that it was simply a tool I used to communicate more effectively, as translating complex concepts from Italian to English can sometimes be challenging. Like humans, AI is not perfect and can make mistakes. However, the contributions themselves were based on my understanding and analysis of the project.

I also noticed that the PR was marked as “0/14 files viewed,” which makes me think the contribution might not have been thoroughly reviewed. I understand that my work might not align perfectly with the project’s current priorities, but I would sincerely appreciate it if you could share the specific issues or misalignments that led to this decision. Understanding this would help me learn and improve for future contributions.

Lastly, I want to say how much I admire this project. Working on it, even for this PR, has been incredibly insightful and has taught me a lot. Thank you for maintaining such a high-quality project, and I hope we might have the chance for a more collaborative and gratifying experience in the future.

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