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

Add explicit input types for vectorized into int form #9741

Merged
merged 2 commits into from Jul 23, 2023

Conversation

sholderbach
Copy link
Member

Description

Don't just use List<Any>, be precise for the vectorized form as well.

User-Facing Changes

More explicit albeit verbose type information in the signature

Don't just use `List<Any>`, be precise for the vectorized form as well.
@sholderbach sholderbach added the type-system Problems or features related to nushell's type system label Jul 19, 2023
Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like being explicite 😊

just a little question and i think we are good to go 😌

crates/nu-command/src/conversions/into/int.rs Outdated Show resolved Hide resolved
@sholderbach
Copy link
Member Author

sholderbach commented Jul 20, 2023

This change is actually not fine with our current type system and what was possible before #9680.
With just the explicit types only homogeneous lists are supported but we actually could support heterogeneous lists with scalars of all supported primitive types. We currently have no way to explicitly express this with the type system, we would either need type unions or something like interfaces/traits for IntoInt.
So at the moment we still need to keep a List<Any> input for heterogenous coercion to work.

With this the PR just clarifies the signatures for documentation
purposes
@sholderbach sholderbach changed the title Narrow input types for vectorized into int form Add explicit input types for vectorized into int form Jul 21, 2023
@sholderbach
Copy link
Member Author

OK pivoted to just adding the explicit signatures for better documentation but keeping the overly broad list<any> form for now to not introduce a breaking change.

@sholderbach sholderbach marked this pull request as ready for review July 21, 2023 18:54
Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this makes sense and will give a better input/output signature table 😊

@sholderbach sholderbach merged commit 4dbdb1f into nushell:main Jul 23, 2023
19 checks passed
@sholderbach sholderbach deleted the narrow-into-int branch July 23, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-system Problems or features related to nushell's type system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants