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

Export tuple type length function #36820

Closed
NHDaly opened this issue Jul 27, 2020 · 6 comments
Closed

Export tuple type length function #36820

NHDaly opened this issue Jul 27, 2020 · 6 comments
Labels
good first issue Indicates a good issue for first-time contributors to Julia needs decision A decision on this change is needed stdlib Julia's standard library

Comments

@NHDaly
Copy link
Member

NHDaly commented Jul 27, 2020

Originally posted by @NHDaly in #32427 (comment)

#32427 added an unexported function, _counttuple(::Type{<:Tuple}) which returns the number of elements in the tuple type.

Can we name this something not private and export it? This is something we use all the time in our code at RelationalAI, where we do a lot of stuff with tuples. It also seems like something that other code will use a lot, like DataFrames, CSV, etc.

In that other comment I suggested arity(::Type{<:Tuple}) as one suggestion. I still like that best, but open to others!

  • arity()
  • tupletypewidth() / tuple_type_width()
  • tupletypecount() / tuple_type_count()
  • tuplearity()
  • tupletypelength() / tuple_type_length()
@NHDaly
Copy link
Member Author

NHDaly commented Jul 27, 2020

This was originally proposed as a fast implementation for fieldcount(::Type{<:Tuple}), but it was deemed that adding methods to fieldcount isn't worth the cost for potential dynamic dispatches: #32495.

@NHDaly NHDaly added needs decision A decision on this change is needed good first issue Indicates a good issue for first-time contributors to Julia stdlib Julia's standard library labels Dec 20, 2020
@sanjibansg
Copy link
Contributor

sanjibansg commented Feb 17, 2021

Can I work in this issue? Please guide me with the required procedures. I am new to this organisation. @NHDaly

@NHDaly
Copy link
Member Author

NHDaly commented Feb 19, 2021

@vtjnash - what do you say? Do you have a preference on any of the names proposed above? ❤️

It would be great to allow @Kahanikaar to pick this up :)

@NHDaly
Copy link
Member Author

NHDaly commented Feb 19, 2021

@Kahanikaar - Thanks for offering to help! Welcome to the community! 😁
I may have been a bit over-zealous in adding the good first issue label: I think it would be a good first issue, but it's slightly blocked on a decision to pick a name. 😊

That said, if you want to give this a shot and open a PR, we can always work-shop the name in the PR!

I think resolving this issue would involve starting from this function:

julia/base/tuple.jl

Lines 17 to 20 in 11cbaf6

# convenience function for extracting N from a Tuple (if defined)
# else return `nothing` for anything else given (such as Vararg or other non-sized Union)
_counttuple(::Type{<:NTuple{N,Any}}) where {N} = N
_counttuple(::Type) = nothing

and renaming it to one of the above choices, and then adding a nice docstring to it! :)

We might also want to add it to the exported names, here:
https://github.com/JuliaLang/julia/blob/master/base/exports.jl

And finally depending on if there are already people relying on the existing name, _counttuple maybe keeping that name as an alias to the new name (i.e. const _counttuple = <new name here>).

Let me know if you have questions!

@sanjibansg
Copy link
Contributor

Initial PR made. Do review @NHDaly @vtjnash

@NHDaly
Copy link
Member Author

NHDaly commented Mar 11, 2021

Closing this, as fieldcount is now statically inferred in the compiler:
#39755 (comment)
#39755 (comment)

@NHDaly NHDaly closed this as completed Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Indicates a good issue for first-time contributors to Julia needs decision A decision on this change is needed stdlib Julia's standard library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants