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

FIX: the math functions can not be used #381

Merged
merged 2 commits into from
Feb 18, 2023

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Feb 18, 2023

before that commit, one could get two errors when useing the script

> use maths/math_functions.nu
Error: nu::parser::expected_keyword (link)

  × Expected keyword.
    ╭─[maths/math_functions.nu:38:1]
 38 │ ## constants
 39 │ let pi = 3.1415926535897932
    · ─┬─
    ·  ╰── expected def or export keyword
 40 │ let e  = 2.7182818284590452
    ╰────

and

> use maths/math_functions.nu
Error: nu::parser::unexpected_keyword (link)

  × Statement used in pipeline.
     ╭─[maths/math_functions.nu:154:1]
 154 │
 155 │     for $i in 0..($n_cols - 1) {
     ·     ─┬─
     ·      ╰── not allowed in pipeline
 156 │         ($x | column2 $i) | scale-minmax $a $b | wrap ($name_cols | get $i)
     ╰────
  help: 'for' keyword is not allowed in
        pipeline. Use 'for' by itself,
        outside of a pipeline.

this PR fixes them by

  • removing the pi and e definitions with let
  • changing the problematic for to an each

This commit mitigates the following error
```bash
> use maths/math_functions.nu
Error: nu::parser::expected_keyword (link)

  × Expected keyword.
    ╭─[maths/math_functions.nu:38:1]
 38 │ ## constants
 39 │ let pi = 3.1415926535897932
    · ─┬─
    ·  ╰── expected def or export keyword
 40 │ let e  = 2.7182818284590452
    ╰────
```

One can use the new `math pi` and `math e` instead.
This commit mitigates the following error
```bash
> use maths/math_functions.nu
Error: nu::parser::unexpected_keyword (link)

  × Statement used in pipeline.
     ╭─[maths/math_functions.nu:154:1]
 154 │
 155 │     for $i in 0..($n_cols - 1) {
     ·     ─┬─
     ·      ╰── not allowed in pipeline
 156 │         ($x | column2 $i) | scale-minmax $a $b | wrap ($name_cols | get $i)
     ╰────
  help: 'for' keyword is not allowed in
        pipeline. Use 'for' by itself,
        outside of a pipeline.
```
@amtoine
Copy link
Member Author

amtoine commented Feb 18, 2023

additional information

key value
version 0.75.1
branch
commit_hash 66398fbf77d03a6c7fd231da157f410d0446132e
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.66.1 (90743e729 2023-01-10)
rust_channel 1.66.1-x86_64-unknown-linux-gnu
cargo_version cargo 1.66.1 (ad779e08b 2023-01-10)
pkg_version 0.75.1
build_time 2023-02-18 11:04:21 +01:00
build_rust_channel release
features database, default, trash, which, zip
installed_plugins gstat

@fdncred
Copy link
Collaborator

fdncred commented Feb 18, 2023

you could/should probably change the lets to consts, since they are constants, right?

@amtoine
Copy link
Member Author

amtoine commented Feb 18, 2023

you could/should probably change the lets to consts, since they are constants, right?

mm not sure it's possible on the latest main, is it? 🤔

when i revert the first commit of this PR, i get

> use maths/math_functions.nu *
Error: nu::parser::expected_keyword (link)

  × Expected keyword.
    ╭─[maths/math_functions.nu:38:1]
 38 │ ## constants
 39 │ let pi = 3.1415926535897932
    · ─┬─
    ·  ╰── expected def or export keyword
 40 │ let e  = 2.7182818284590452
    ╰────

so let's try adding an export to these

>_ use maths/math_functions.nu *
Error: nu::parser::parse_mismatch (link)

  × Parse mismatch during operation.
    ╭─[maths/math_functions.nu:38:1]
 38 │ ## constants
 39 │ export let pi = 3.1415926535897932
    ·        ─┬─
    ·         ╰── expected def, def-env, alias, use, or env keyword
 40 │ export let e  = 2.7182818284590452
    ╰────

and we get the exact same two errors when replacing let by const each time 😕

you tell me 😋

@fdncred
Copy link
Collaborator

fdncred commented Feb 18, 2023

@kubouch What are we missing here? I thought you could use loose const in modules like:

const pi = 3.1415926535897932
const e  = 2.7182818284590452

This is what I get here

use math_functions.nu * 
Error: nu::parser::expected_keyword (link)

  × Expected keyword.
    ╭─[math_functions.nu:38:1]
 38 │ ## constants
 39 │ const pi = 3.1415926535897932
    · ──┬──
    ·   ╰── expected def or export keyword
 40 │ const e  = 2.7182818284590452
    ╰────

Maybe all consts have to be inside of defs?

@fdncred
Copy link
Collaborator

fdncred commented Feb 18, 2023

But @amtoine, we also don't need these let/const anymore anyway because we added math pi and math e. Maybe you already said that and I wasn't catching on?

@amtoine
Copy link
Member Author

amtoine commented Feb 18, 2023

i'm confused 😆

i've removed e and pi from maths/math_functions.nu, 'cause one can use math e and math pi instead

do i miss something? 😕

@amtoine
Copy link
Member Author

amtoine commented Feb 18, 2023

But @amtoine, we also don't need these let/const anymore anyway because we added math pi and math e. Maybe you already said that and I wasn't catching on?

so i'd say i've said that yeah 😋

@kubouch
Copy link
Contributor

kubouch commented Feb 18, 2023

Neither const not let are supported in modules. I was planning to try it but didn't get to it yet.

@fdncred
Copy link
Collaborator

fdncred commented Feb 18, 2023

ok, i think we're good here. we have those constants as math commands.

Neither const not let are supported in modules.

Thanks for the clarification

@fdncred fdncred merged commit fa5f262 into nushell:main Feb 18, 2023
@amtoine amtoine deleted the fix/math-functions-syntax-errors branch February 19, 2023 09:55
@amtoine
Copy link
Member Author

amtoine commented Feb 19, 2023

coool 😋

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.

3 participants