Skip to content

Update internal use of decimal to float#10333

Merged
sholderbach merged 9 commits intonushell:mainfrom
sholderbach:overfloat
Sep 13, 2023
Merged

Update internal use of decimal to float#10333
sholderbach merged 9 commits intonushell:mainfrom
sholderbach:overfloat

Conversation

@sholderbach
Copy link
Copy Markdown
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
Copy Markdown
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 notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes label Sep 12, 2023
@sophiajt
Copy link
Copy Markdown
Contributor

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

@sholderbach
Copy link
Copy Markdown
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
Copy Markdown
Member

amtoine commented Sep 13, 2023

this PR does not yet remove the latter.

soon 😏

@sholderbach
Copy link
Copy Markdown
Member Author

@jntrnr and @amtoine

Now no more decimal around :P

@sholderbach sholderbach merged commit bbf0b45 into nushell:main Sep 13, 2023
@sholderbach sholderbach deleted the overfloat branch September 13, 2023 21:53
@amtoine
Copy link
Copy Markdown
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! notes:breaking-changes 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.

3 participants