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

Change to use Tuple type in type params #66

Open
oxinabox opened this issue Sep 17, 2019 · 10 comments
Open

Change to use Tuple type in type params #66

oxinabox opened this issue Sep 17, 2019 · 10 comments
Labels
breaking a change that if made would be breaking decision Something that needs to be decided
Milestone

Comments

@oxinabox
Copy link
Member

It is a bit uglier but then can dispatch on them.

And the wildcard becomes Any
(I.e <:Any since tuples are covarient)

More generally relax things so it can be any thing allowed in a type-param.
But Symbol is going to be best since then can have kwargs getindex etc.

Needed for #43

@oxinabox oxinabox added breaking a change that if made would be breaking decision Something that needs to be decided labels Feb 14, 2020
@oxinabox oxinabox added this to the 1.0 milestone Feb 14, 2020
@nickrobinson251
Copy link
Contributor

Any more thoughts on this?

I'm thinking about if we can tag 1.0, and tempted to suggest yes, unless there are plans to make the change anytime soon.

@Tokazama
Copy link
Contributor

Are there any downsides to this besides it being visually less appealing

@oxinabox
Copy link
Member Author

No, but it is
significantly less visually appealing.

@mcabbott
Copy link
Collaborator

Could you clarify what's proposed? It would be possible leave dimnames(A) and constructors alone, even if the storage is different, and :_ is translated to/from Any. Then summary(A) could print the constructor, like view does, for visual appeal.

@Tokazama
Copy link
Contributor

I think we can overcome ugly with overloading show but I'm concerned about hurting performance or easy to use syntax for constructing things. Otherwise we get to dispatch on names

@oxinabox
Copy link
Member Author

Dispatch on partial names.
We can already dispatch on names.
And you almost certainly have to use the ugly form to write it.

I hope we have enough @ballocated tests in the package to ensure this does not impact performance

@mcabbott
Copy link
Collaborator

I guess you could write definitions likef(x::NamedType(:a, :_)) = "ok" with a function like NamedType(sy::Symbol...) = Nda{<:Tuple{map(s -> s===:_ ? Any : s, sy)...}}.

@oxinabox
Copy link
Member Author

An interesting possibility.

@Tokazama
Copy link
Contributor

I agree Tuple{} isn't as nice as () but so far I see this change as a good thing. Once it's set up we should only need to deal with the ugly syntax problem when trying to take advantage of individual names with a Tuple.

That being said, it might be good to let this incubate a bit longer. It's obvious not everyone loves the change and no one has expressed a strong opinion in favor of it. It's always possible that an even better solution is identified or some lurking issue with using Tuple is discovered. No need to rewrite this code twice.

@Tokazama
Copy link
Contributor

What if we use Unitful.Dimensions inside a Tuple? I know it would be a bit of a paradigm shift, but it would give us a lot of the capabilities we are looking for here and it's already integrated in so many parts of Julia that it would be easy to get others to adopt extensions to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking a change that if made would be breaking decision Something that needs to be decided
Projects
None yet
Development

No branches or pull requests

4 participants