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

feat(ext.commands): add LargeRange #857

Closed
wants to merge 13 commits into from

Conversation

EmmmaTech
Copy link
Contributor

@EmmmaTech EmmmaTech commented Nov 7, 2022

Summary

This PR adds support for large numbers (numbers bigger than 2 ** 53 or smaller than -2 ** 53) in a version of commands.Range named commands.LargeRange.

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running task lint
    • I have type-checked the code by running task pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@shiftinv shiftinv added t: enhancement New feature s: needs review Issue/PR is awaiting reviews labels Nov 8, 2022
@onerandomusername
Copy link
Member

After reading the comments that @shiftinv has left, I think the best course of action is to leave Range as it currently is. Instead, add a new class called LargeRange (name can be bikeshedded), which switches between the two if needed. In addition, this is large by default, but can switch to float or int if the max is below the threshold.

LargeFloat would not be needed, as it would be a part of LargeRange (again, bikeshed name)

So to summarize, my proposed changes are:

  • keep existing implementation of Range the same
  • make LargeRange function like existing Range class, but with automatic large support and default to large.
  • remove LargeFloat
  • create and add any necessary conversion errors

@EmmmaTech EmmmaTech changed the title feat(ext.commands): add large number support in Range & LargeFloat feat(ext.commands): add LargeRange Feb 22, 2023
@shiftinv
Copy link
Member

shiftinv commented Mar 26, 2023

As of pyright 1.1.299, using non-generic metaclasses with __getitem__ in type expressions no longer works (which, to be fair, wasn't part of the spec).
This means most of Range and String will have to be rewritten to make use of typing.Annotated in some way.
I'll close this PR for the time being, since this will likely end up being added as part of said rewrite; thank you for the contribution though :)

@shiftinv shiftinv closed this Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: needs review Issue/PR is awaiting reviews t: enhancement New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants