Skip to content

Fix type hints #9

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

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Fix type hints #9

merged 1 commit into from
Apr 21, 2023

Conversation

MaxGyver83
Copy link
Contributor

I'm playing around with Cython to see if this can improve the performance. Cython is strict about types and gave me some errors, for example:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "main.pyx", line 123, in main.main
    varLetters = getVariableLetters(letters_L1, staticLetters)
TypeError: Argument 'staticLetters' has incorrect type (expected str, got tuple)

This pull request fixes all TypeErrors Cython complained about.

@Glitchy-Tozier
Copy link
Owner

  1. Thank you for the PR :)
  2. Just to make sure you know about the feature: Are you aware that pypy speeds up the script by a lot? If no, see README.md
  3. Most of the changes make perfect sense. I'll individually comment on the two areas that I don't understand.

@MaxGyver83
Copy link
Contributor Author

  1. Thank you for the PR :)
  2. Just to make sure you know about the feature: Are you aware that pypy speeds up the script by a lot? If no, see README.md
  3. Most of the changes make perfect sense. I'll individually comment on the two areas that I don't understand.
  1. My pleasure.
  2. Yes, I'm aware of this and I only use pypy3. But it still needs a lot of time. On my laptop, 13 minutes with the default configuration. When I play around with the configuration, I need many runs. Then it sums up. That's why faster would be better :-D
  3. OK, I'll answer your comments.

@Glitchy-Tozier Glitchy-Tozier merged commit 6ad91f7 into Glitchy-Tozier:main Apr 21, 2023
@Glitchy-Tozier
Copy link
Owner

Merged 👍🏻

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