Skip to content

math.big: rework function naming and documentation#18890

Merged
medvednikov merged 5 commits intovlang:masterfrom
phoreverpheebs:mathbig_cosmetic
Jul 19, 2023
Merged

math.big: rework function naming and documentation#18890
medvednikov merged 5 commits intovlang:masterfrom
phoreverpheebs:mathbig_cosmetic

Conversation

@phoreverpheebs
Copy link
Copy Markdown
Contributor

@phoreverpheebs phoreverpheebs commented Jul 17, 2023

Overview

  • A few functions are renamed to allow for a more extensible, legible and standardized scheme; i.e. mathematical functions (isqrt, gcd, ...) remain as they are, however methods such as left_shift and right_shift are changed to be more verbose and clear -- allowing future left_rotate and right_rotate methods (more legible than e.g. rrotate). The renaming of lshift and rshift is applied internally as well for consistency.

  • The function bit_length is deprecated as it seems to have been made public by accident in math.big: Improve multiplication performance #15200. Instead use of bit_len is encouraged.

  • Renaming is also done in big.js.v to better fit the naming of the rest of the module; warranting deprecation of renamed functions.

  • In the base arithmetic operations the first operand is also renamed to match their respective formal names (see this discussion on math.stackexchange).

  • Documentation is adjusted in certain situations where the declared operand name does not match the documented name. Missing documentation for overridden arithmetic operators is also added for clarity.

Notes

  • isqrt must remain un-renamed, so as to not be mixed up with a future decimal precision return type function, which shall be named sqrt.
  • inc and dec are common enough to be left alone in this PR.
  • Similarly, abs and neg are common short forms of absolute values and negations, therefore they remain as they are.
  • bitwise_* methods may warrant further discussion in this PR.
  • big.Integer to string methods binary_str, hex and radix_str should be discussed for a general scheme, such as bin_str, hex_str and radix_str being longer to be visually better separated from the predetermined bases.

🤖 Generated by Copilot at 075a6a1

This pull request renames several functions and parameters in the math/big module to improve clarity, consistency, and readability. It also adds deprecated annotations and wrapper functions for the old names to ensure backward compatibility. The changes affect the files big_test.v, big.js.v, division_array_ops.v, exponentiation.v, integer.v, special_array_ops.v, and their corresponding test files.

🤖 Generated by Copilot at 075a6a1

  • Rename lshift and rshift functions to left_shift and right_shift for clarity and consistency with Integer methods (link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link)
  • Rename divmod function to div_mod to follow snake_case convention and be consistent with Integer method of the same name (link,link)
  • Rename b_and, b_or, and b_xor functions to bitwise_and, bitwise_or, and bitwise_xor for descriptiveness and consistency with Integer methods of the same names (link,link,link)
  • Rename fact function to factorial_int for descriptiveness and consistency with factorial function that takes a Number argument (link)

@JalonSolov
Copy link
Copy Markdown
Collaborator

JalonSolov commented Jul 17, 2023

Most builtin types support a .hex() method, which returns a string with the hex representation of the value. I'd suggest just using hex instead of hex_str to match.

In the same vein, the other _str() methods need to be examined.

@phoreverpheebs
Copy link
Copy Markdown
Contributor Author

Oh okay, I only knew about .str(). So it's just a matter of seeing how we can change the rest of the str functions to work best with the builtin compliant ones. I'm assuming .bin() wouldn't clash with anything? But then again it could become a problem in the future or with how intuitive it is to understand what it does.

@JalonSolov
Copy link
Copy Markdown
Collaborator

The builtin types don't have a bin() method...

error: unknown method or field: `int literal.bin`.
3 possibilities: `hex`, `hex_full`, `str`.
    5 | import math
    6 | 
    7 | println(5.bin())
      |           ~~~~~

So... as long as the name makes sense, it should be fine.

Bytes do have an extra method... ascii_str(), which is different from str(). So bin_str() might make some sense... not sure on that one.

@phoreverpheebs
Copy link
Copy Markdown
Contributor Author

bin_str sounds alright. also i'll fix the identifier errors when i'm at my computer

@phoreverpheebs
Copy link
Copy Markdown
Contributor Author

@hungrybluedev thoughts on bin_str deprecating binary_str?

@hungrybluedev
Copy link
Copy Markdown
Contributor

@hungrybluedev thoughts on bin_str deprecating binary_str?

Seems fine as long as we include the full word “binary” in the documentation somewhere. People should be able to find it through page search.

@medvednikov medvednikov merged commit a49b8f2 into vlang:master Jul 19, 2023
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.

4 participants