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

Update internal use of decimal to float #10333

Merged
merged 9 commits into from Sep 13, 2023
Merged

Conversation

sholderbach
Copy link
Member

@sholderbach sholderbach commented Sep 12, 2023

Description

We made the decision that our floating point type should be referred to as float over decimal.
Commands were updated by #9979 and #10320

Now make the internal codebase consistent in referring to this data type as float.

Work for #10332

User-Facing Changes

decimal has been removed as a type name/symbol.

Instead of

def foo [bar: decimal] decimal -> decimal {}

use

def foo [bar: float] float -> float {}

Potential effect of SyntaxShape's Display implementation now also referring to float instead of decimal

Details

  • Rename SyntaxShape::Decimal to Float
  • Update Display for SyntaxShape to float
  • Update error message + fn name in dataframe code
  • Fix docs in command examples
  • Rename tests that are float specific
  • Update doccomment on SyntaxShape
  • Update comment in script

Tests + Formatting

Updates the names of some tests

@sholderbach sholderbach added the naming-things 🚲 🛖 Working towards consistent naming. No bikeshedding brigade please! label Sep 12, 2023
@sholderbach
Copy link
Member Author

Update on the impact of changing the display name of SyntaxShape::Float: The IDE flags use FlatShape which already refers to it as float. (So no need for tooling release synchronization)

Affected would be the help and scope output. If some one depends on a particular signature this will be a breaking change.

@sholderbach sholderbach added the pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes label Sep 12, 2023
@sophiajt
Copy link
Member

@sholderbach - is the type/shape name the user types now float also?

@sholderbach
Copy link
Member Author

@jntrnr

is the type/shape name the user types now float also?

It already supported both float and decimal

this PR does not yet remove the latter.

@amtoine
Copy link
Member

amtoine commented Sep 13, 2023

this PR does not yet remove the latter.

soon 😏

@sholderbach
Copy link
Member Author

@jntrnr and @amtoine

Now no more decimal around :P

@sholderbach sholderbach merged commit bbf0b45 into nushell:main Sep 13, 2023
19 checks passed
@sholderbach sholderbach deleted the overfloat branch September 13, 2023 21:53
@amtoine
Copy link
Member

amtoine commented Sep 14, 2023

Now no more decimal around :P

loving it 💪

hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
# Description
We made the decision that our floating point type should be referred to
as `float` over `decimal`.
Commands were updated by nushell#9979 and nushell#10320

Now make the internal codebase consistent in referring to this data type
as `float`.

Work for nushell#10332

# User-Facing Changes

`decimal` has been removed as a type name/symbol. 

Instead of 
```nushell
def foo [bar: decimal] decimal -> decimal {}
```
use 
```nushell
def foo [bar: float] float -> float {}
```

Potential effect of `SyntaxShape`'s `Display` implementation now also
referring to `float` instead of `decimal`

# Details
- Rename `SyntaxShape::Decimal` to `Float`
- Update `Display for SyntaxShape` to `float`
- Update error message + fn name in dataframe code
- Fix docs in command examples
- Rename tests that are float specific
- Update doccomment on `SyntaxShape`
- Update comment in script

# Tests + Formatting
Updates the names of some tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
naming-things 🚲 🛖 Working towards consistent naming. No bikeshedding brigade please! pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants