Open dams opened 11 years ago
Well, trying to find a way to support Moo is definitely on my list. I was looking at MooX::Types::MooseLike (somewhat doubtfully), but perhaps this is another alternative. I won't have any time until after YAPC, but I'll look into this first chance I get.
Thx for the tip!
Once #34 is done, Type::Tiny can definitely be added to the set of introspected type systems. However, allowing lexical scalars as types may cause type signature parsing to become ambiguous. That'll have to be thought through some.
my $NUM = "Type::Tiny"->new(
name => "Number",
constraint => sub { looks_like_number($_) },
message => sub { "$_ ain't a number" },
);
method add( $NUM $left, $NUM $right ) {
...
}
Didn't have more than 2 hours to spend... However, this simple hack makes the standard types supported: https://github.com/dams/method-signatures/commit/d1eb0e3e051f7b1f311aeaaf61e9de4e2fba1b28 As this test demonstrates it: https://github.com/dams/method-signatures/commit/20a536784946a324301c536519ff1361d1012646 Not sure how you guys want Type::Tiny types to be enabled. An option on M::S import ? or should Type::Tiny be checked before Any::Moose ? Also, I'll try to make class/rolle based types work. About custom types, I think that it's just a matter of the user adding it to the Type::Tiny registry that we would have built for M::S, so an ad-hoc method to do just that should be enough (so no need to parse custom types as scalar in the method signature). Any hint, help or guidance welcome.
Please see Pull Request #83. It's probably better to continue discussion over there and close this bug.
Well, I've finally found the time to review Type::Tiny, and I must say: it seems to me like it might be worthwhile to consider just using Type::Tiny for everything. I can't see anything that Moose can do that Type::Tiny can't, including some things that Mouse can't do. It's small, it's fast, it would basically moot #34, it works with Moose, Mouse, and Moo ... where's the downside to switching over to it for everything?
AFA this:
my $NUM = "Type::Tiny"->new(
goes, that's an anonymous type, and I don't see any reason to allow those in method signatures.
We get Types::Standard available by default with this I assume.
On Thu, Jul 4, 2013 at 2:11 PM, Buddy Burden notifications@github.comwrote:
Well, I've finally found the time to review Type::Tiny, and I must say: it seems to me like it might be worthwhile to consider just using Type::Tiny for everything. I can't see anything that Moose can do that Type::Tiny can't, including some things that Mouse can't do. It's small, it's fast, it would basically moot #34https://github.com/schwern/method-signatures/issues/34, it works with Moose, Mouse, and Moo ... where's the downside to switching over to it for everything?
AFA this:
my $NUM = "Type::Tiny"->new(
goes, that's an anonymous type, and I don't see any reason to allow those in method signatures.
— Reply to this email directly or view it on GitHubhttps://github.com/schwern/method-signatures/issues/81#issuecomment-20458294 .
@barefootcoder I agree, I also think that using Typ::Tiny everywhere is a great choice. @rjattrill: in my PR yes you get types::standard by default, and you can also load additional types at "use" time.
We get Types::Standard available by default with this I assume.
Well, my suggestion is that you get it by default, by not-default ... you just get it, period. I may try some benchmarks with the Moose type vs Type::Tiny, because I suspect that Type::Tiny will be faster, but I'll need to prove it. But, assuming Type::Tiny is faster, it appears to be just as featureful, you can create new types with it just as easily, it handles coercions just as well, and it works seamlessly with Moose (as well as all the others). So I'm having difficulty imagining why, as a user, you'd ever not want to use it.
I guess what I'm asking is, can anyone think of any reason why someone would prefer using Moose types over Type::Tiny?
Ah, here's one: what if you already have a bunch of Moose types built up--subtypes and enums and whatnot. You have your own type registry going, and you don't want to switch to Type::Tiny (why not, other than just not having the time to do it, I can't imagine, but that's not for us to say). Can Type::Tiny handle user-defined types, if those types are defined via Moose?
In my opinion, M::S should be flexible enough to be used with Moose types or Types::Tiny, and maybe others.
As you said, some people may want to prefer Moose types because they work on existing code that uses it and they cannot migrate, or they use specific features of Moose types (if any).
However, one big advantage of Type::Tiny is the fact that you can have more than one Type::Registry, instead of the global namespace for Moose types, which forces you to heavily namespace your types.
Also, what about backward compatibility ? it would be a shame that a new version of M::S breaks your code and forces you to migrate to Type::Tiny
Last but not least, what if other types implementation emerge ? we've seen Moose, Mouse, then Moo (I won't mention the other Mo* stuff...) And it's difficult to see in advance which one are going to be used ultimately.
So if there can be a clever way to be compatible with any type implementation, I'm all for it.
@dams: Let me take your points one at a time:
... some people may want to prefer Moose types because they work on existing code that uses it ...
Using Moose types for your attributes and Type::Tiny for your method parameter types should present no problems as long there aren't any differences (even subtle ones) between the two implementations for the same type, which I don't believe there are.
... or they use specific features of Moose types (if any).
But that's what I'm saying: I believe that the way @tobyink has designed Type::Tiny is that it both is and always will be feature-compatible with Moose types.
Also, what about backward compatibility ?
Given feature compatibility, I can't see any way that this change would break backcompat.
Last but not least, what if other types implementation emerge ? we've seen Moose, Mouse, then Moo (I won't mention the other Mo* stuff...)
Well, exactly. Type::Tiny is a way to get off this demonic merry-go-round. If we say, "we don't care what crazy system you're using, and whatever types it wants to use, we're going to use Type::Tiny" then we don't give a crap what the next type craze is.
Now, I'm making some assumptions here (faster implementation, full feature compatibility, commitment to maintain such, etc). I'm going to have to do some investigation to verify these things. I could be wrong, and, if I am, then the question is moot and we'll have no other choice than to support multiple implementations. But, if I'm right ...
Why not?
Here's a few random points...
Types::TypeTiny::to_TypeTiny( SomeMooseXTypesType )
.ArrayRef
is a Type::Tiny type, and Foo
is a MooseX::Types type, then ArrayRef[Foo]
ought to "just work"."Type::Registry"->for_me
from within Method::Signatures will always return the same Type::Registry object, so you lose the benefit of per-package type registries. You want "Type::Registry"->for_class($class_being_compiled)
. (Or better, the DWIM stuff in the next release.)FWIW, I would be delighted if M::S could drop all it's Moose/Mouse dependencies.
I have nothing against that worthy ecosystem, except that it makes using M::S a much heavier proposition and correspondingly less viable in certain types of development.
If Type::Tiny can reduce the M::S memory footprint, I'm all for it!
Damian
@tobyink: Thanks for all the detailed explanations. I'm just waiting to hear from @schwern at this point to get his input. I may try to fiddle with the idea on a branch, as a proof of concept.
One question/clarification: You talk about the automatic translation of Moose types to Type::Tiny types. Let's say that a user creates one or more new Moose types, using subtype
or enum
or what-have-you. Will I be able to write the type checking for MS in such a way that, if these new types are used as parameter types in sigs, they'll just be automagically converted to appropriate Type::Tiny types? 'Cause, if that could work, then I really can't see any reason not to just go with Type::Tiny for everything here.
@thoughtstream:
I have nothing against that worthy ecosystem, except that it makes using M::S a much heavier proposition and correspondingly less viable in certain types of development.
Well, remember that MS doesn't depend on (or drag in) Moose, which is the heavy one. MS will never use Moose unless you use it first. Mouse is a much smaller dependency. Now, that having been said, here's what perlbloat
tells me on 5.14.4:
Moose: 30.4Mb Mouse: 7.7Mb Type::Tiny: 2.9Mb
So, while Mouse is a hell of a lot better than Moose (and I don't think worthy of being considered a much heavier proposition), Type::Tiny is even better.
I continue to wait for @schwern, but I'm really feeling more and more like Type::Tiny is the way to go here. Can anyone offer any counterindication to my enthusiasm?
@barefootcoder as per the @tobyink remarks, I now understand your approach. The major drawback for me was if a user has a ton of Moose types that he couldn't use in Type::Tiny. If that's transparently handled by T::T, then it's all good. And indeed we can consider T::T as the central compat layer for MOP implementations. But now, I'm tempted to try Specio types in Method::Signatures :)
@tobyink
About the Type::Registry issue: in my patch I always use the same type registry because I wanted all custom types to be available accross all code that uses M::S. A little bit like the global Moose type registry.
If I change the patch to do what you suggest, it means that every class using M::S with custom T::T types will have to load these custom types to their registry each time they use Method::Signatures
. Am I right ?
Maybe the solution is to ask people to put their custom types in a single registry that they pass to Method::Signature at use time ? like use Method::Signature registry => $reg
. Exposing the registry would be nicer than the load_types
option I implemented. But then I'm worried about the syntax complexity.
About DWIM stuff: from my tests, I think this already works fine with my patch. If you look at type_check_type_tiny.t line 41 you'll see that I've commented the line just because the error message is different and the test fails. But it properly recognizes Foo::Bar
as being InstanceOf['Foo::Bar']
. Is it a strange side effect?
Anyway I'll wait for that DWIM release, and in the mean time try to improve my patch.
@barefootcoder: Shall I give it a new try with a new PR removing Mo*se altogether and relying only on TT ?
Let's say that a user creates one or more new Moose types, using subtype or enum or what-have-you. Will I be able to write the type checking for MS in such a way that, if these new types are used as parameter types in sigs, they'll just be automagically converted to appropriate Type::Tiny types?
The following test script passes (except the TODO) in the latest dev release of Type-Tiny...
use strict;
use warnings;
use Test::More 0.96;
use Test::TypeTiny;
use Moose;
use Moose::Util::TypeConstraints qw(:all);
use Type::Utils 0.015 qw(dwim_type);
# Creating a type constraint with Moose
subtype "Two", as "Int", where { $_ eq 2 };
my $two = dwim_type("Two");
my $twos = dwim_type("ArrayRef[Two]");
isa_ok($two, 'Type::Tiny', '$two');
isa_ok($twos, 'Type::Tiny', '$twos');
should_pass(2, $two);
should_fail(3, $two);
should_pass([2, 2, 2], $twos);
should_fail([2, 3, 2], $twos);
# Creating a type constraint with MooseX::Types
{
package MyTypes;
use MooseX::Types -declare => ["Three"];
use MooseX::Types::Moose "Int";
subtype Three, as Int, where { $_ eq 3 };
}
# Note that MooseX::Types namespace-prefixes its types.
my $three = dwim_type("MyTypes::Three");
my $threes = dwim_type("ArrayRef[MyTypes::Three]");
TODO: {
local $TODO = 'this probably needs fixing';
isa_ok($three, 'Type::Tiny', '$three');
}
isa_ok($threes, 'Type::Tiny', '$threes');
should_pass(3, $three);
should_fail(4, $three);
should_pass([3, 3, 3], $threes);
should_fail([3, 4, 3], $threes);
done_testing;
I wouldn't like to say that everything would be completely transparent. There are always going to be weird edge cases.
@dams:
Shall I give it a new try with a new PR removing Mo*se altogether and relying only on TT ?
Well, I'd definitely like to have something along those lines in a branch. I'm not sure how you could leave the existing pull request intact and create a whole new one though ... maybe your git-fu is stronger than mine though. :-)
@tobyink:
The following test script passes (except the TODO) in the latest dev release of Type-Tiny...
Wow ... impressive. I'm definitely getting excited about the possibilities here. :-)
@barefootcoder creating a new PR is easy :) no problem with that. I'll try to mock up something.
creating a new PR is easy :) no problem with that.
Well, you'll have to tell me how to do it sometime. :-) My experience is, once I create a pull request on my fork, every commit I do to that fork from then on goes into the existing pull request. So I'm not sure how to make it so that you have two different PRs for the same repo. But I'm sure it's possible.
My experience is, once I create a pull request on my fork, every commit I do to that fork from then on goes into the existing pull request.
Branches. You can commit different threads of work to different branches, and then create a pull request per branch.
@barefootcoder as @tobyink said, I usually don't commit on master on my repo (forked from upstream, that mean in this case, here). I usually create a branch, commit into it, and then create the PR from within it.
@barefootcoder @tobyink @schwern : so, took me some time, but I've come up with a clen PR to have M::S rely purely on Type::Tiny. Check it out: PR #85
Let me know if you like it
Branches. You can commit different threads of work to different branches, and then create a pull request per branch.
Ah, yes, that makes sense. Same branch, same pull request; different branches, different pull requests.
so, took me some time, but I've come up with a clen PR to have M::S rely purely on Type::Tiny. Check it out: PR #85
Excellent, @dams. Thx for the quick turnaround. Hopefully I can take a look this weekend.
Sorry for the late commentary. I love the idea of including Type::Tiny, I think it's a great type ecosystem. I have a problem with using only Type::Tiny.
For a given project, I tend to write custom types. If I'm using Mouse or Moose, I'm going to write them using Mouse or Moose. If I use Method::Signatures, I'm going to expect it to use those types. If MS only uses Type::Tiny then I need one type system for my OO stuff and one for my signatures.
I have exactly this problem, in reverse, with Gitpan. Gitpan is using Moo and thus Type::Tiny. Method::Signatures is using Mouse types. I have a bunch of custom types and coercions that I can't use in signatures. Annoying.
This is why I proposed #34 to choose which type system to use on a per class basis. If your class is using Moose, use Moose types when compiling signatures for that class. If it's using Mouse, use Mouse. Otherwise look for a function that returns a Type::Tiny object. If there's no function, use Mouse (because it's cheap and fast and already loaded). Also offer a way to make it explicit with use Method::Signatures types => "Type::Tiny"
or something.
Yeah, it's over-complicated, but we're stuck with multiple type systems until the Mo* folks figure it out.
For what it's worth, Type::Utils (bundled with Type::Tiny) provides a function dwim_type
which can be used like this:
dwim_type("ArrayRef[Foo]", for => "Local::Class");
This does a DWIM type lookup from the perspective of Local::Class, using this technique:
It could probably be tweaked. e.g. or the Moose/Mouse lookups, it might be more sensible instead of checking that they are loaded, to check if Local::Class is built using them.
@tobyink It would make me deliriously happy to offload type guessing to some other module. Well volunteered.
The order I would suggest is...
use Method::Signatures { types => "Moose" }
I just got a relevant test case passing last night. It defines a Moose class and a Mouse class. It also defines a Moose type constraint called FortyFive
and a Mouse type constraint called FortyFive
which have conflicting definitions. dwim_type()
will provide the correct definition of FortyFive
depending on which class the type constraint is requested for.
The one edge case where it has to make a judgement call is: Moose and Mouse are both in memory, and each define a type constraint with the desired name, and the class which is requesting a type constraint is neither a Moose nor a Mouse class. In this case, it prefers the Moose type constraint.
@tobyink Great work!
As to the selection in the ambiguous case, there are problems with the "use Moose if it's in memory" plan. First, it assumes that Moose and Mouse types act equivalently, which is true of the core types but falls on its face for any custom types.
Second, and far worse, is you can't know which it is going to pick. Since it's based on the internal state of the entire process, anything loading Moose might cause your class to start using Moose types. Similarly, if your non-affiliated class is using Mouse types today, something in your dependency chain might use Moose tomorrow. With no warning your class has switched to Moose, your custom Mouse types no longer work, what a PITA to debug.
These are all hateful things about Any::Moose, which I think we can call agree sucks on rice, and it would be silly to keep it in the plan.
There is backwards compatibility to consider. I'm going to split backwards compatibility into two cases. The first is an unaffiliated package using standard Moose types. Since Moose and Mouse have the same standard types, this works fine and should continue to work fine.
The second is a package using custom Moose or Mouse types. This is already unreliable. I'm ok with breaking it if it means putting in a new system that is sensible. For the record, I use custom Mouse types with MS all the time.
I propose that the fallback case is simply Mouse. Why? It's deterministic, there's no action at a distance. Why Mouse and not Moose? MS already loads Mouse and Moose is a pig. It is backwards compatible for the "standard types" case.
An alternative is to rewrite Method::Signatures using Moo and have Type::Tiny be the fallback. This is advantageous if Method::Signatures has to load Type::Tiny just to resolve any types. Since Types::Standard will be loaded by Method::Signatures it can be made backwards compatible for the "standard types" case. And, if @tobyink is going to stick around we can take full advantage of his knowledge of types.
The important thing is that the resolution of the ambiguous type case be made unambiguous and lexically predictable. No action at a distance. No changes in your dependency chain affecting the choice of type system.
We can also add some documentation about how to make types unambiguous throughout your project, because not everything is a class.
package MyProject::Signatures;
use Method::Signatures ();
sub import {
my $caller = caller;
return Method::Signatures->import({ into => $caller, types => "Moose" });
}
BTW Currently the biggest, fattest drag on Method::Signatures is PPI. If you want Method::Signatures to load itself and compile signatures faster, that is the target.
Just to be clear, the ambiguous case we're talking about is where Local::Class contains a method signature with a type constraint "Foo", and:
Writing all this out in long hand has led me to the perfect solution (hurrah!)... in this situation it should accept any value that would be accepted by either Moose or Mouse. That is, a union type constraint.
Of course there is still the possibility of load-order-related weirdness. What happens if Moose isn't in %INC, but gets loaded later? At some point you just have to say "you can do this, but if it breaks the only guarantee is we'll let you keep both halves".
Good idea to clarify, because my assumptions for the ambiguous case turn out to be broader. Let me lay out my conditions for the ambiguous case, which are just the inverse of how to figure out the type system.
This would be done in the import call by the author.
This is most likely to arise in a procedural library, though plenty of people will want to write OO by hand. If the module uses any of these we can make an educated guess as to their preferred type system.
Like if Types::Standard is imported.
And that's about it.
It is not conditional on what is in %INC, and it is not conditional on whether Moose or Mouse define a type constraint. The reason? Action at a distance. Some other module loading Mouse or Moose or whatever should not affect the choice of types in an unrelated module, that is the Any::Moose problem.
Using the union of matching type constraints is very clever... and it only adds to the ambiguity. Not only has Method::Signatures silently chosen a type system for you, but now it's silently mashed two types together. What a nightmare to debug!
It gets worse. Method::Signatures does not yet support coercion, but it most certainly will as soon as I have time. What if both types have coercions, which do you choose?
It gets worse. What if someone writes type-ambiguous code, and their call stack does not load Moose but something loads Mouse. Ok, they get Mouse types. A few months later, their code mysteriously breaks. Why? After a lot of frustrating debugging, including diving into the guts of Method::Signatures (or hopefully turning on helpful debugging messages) they find out their code is now using Moose types (or a union of a Moose and Mouse types). Why? Because some module in their call stack started using Moose in an upgrade. Or maybe one stops using Mouse, that will change everything again. This sort of situation happened to Gitpan recently when Net::Github::V3 switched to Moo and all my roles and subclasses broke.
The more I use Types::Standard the more I'm appreciating it's local type constraints. And the more I'm finding Moose and Mouse's global type constraints problematic for the same reasons anything global is problematic.
When it comes to guessing the author's intent and you have no information to go on, don't guess. Provide a usable and unambiguous default. Simplest, and lightest, solution is to use whatever Method::Signatures is already using, and to explicitly define it in the docs in case we change ourselves internally.
Hmmm... I think you've convinced me that dwim_type
ought to not fall back to using Moose/Mouse types unless one of the following is true:
In pretty much all the tests for dwim_type
the caller explicitly uses either Moose or Mouse, so I think I should be able to make that change without breaking the current Type::Tiny test suite. If so, I'll make the change shortly. If it breaks any of the current tests it will take a while longer. (I've made a commitment to not break anything in the current test suite without giving 6 months' notice.)
On 8/18/14 12:35 PM, Toby Inkster wrote:
Hmmm... I think you've convinced me that |dwim_type| ought to not fall back to using Moose/Mouse types unless one of the following is true:
- the caller class/role uses Moose/Mouse; or
- a fallback to Moose/Mouse is explicitly requested.
Huzzah!
In pretty much all the tests for |dwim_type| the caller explicitly uses either Moose or Mouse, so I think I should be able to make that change without breaking the current Type::Tiny test suite. If so, I'll make the change shortly. If it breaks any of the current tests it will take a while longer. (I've made a commitment to not break anything in the current test suite without giving 6 months' notice.)
There's a bunch of options if compatibility is a concern. For one, %options! dwim_type() takes %options. Add check_if_loaded and default it to [qw(Moose Mouse)]. Then Method::Signatures can pass in an empty list.
I'm very pleased to see some movement here. I've really been meaning to dive into this for many months now, but several things have conspired to keep me away. Some of that is RL beating me over the head, but a healthy chunk of it is just that thinking about how to test all this stuff makes my brain hurt.
I think the first thing we need is some serious test coverage to make sure that the right types always get loaded. If we could achieve that, we could reach a confidence that using only Types::Tiny actually could/would work. I think it could, and I think @tobyink is working hard to make sure it does. And I'm willing to take the plunge and try it out, if we have some solid test cases to demonstrate that it really does work and catch any breakage.
@schwern, it sounds like you may have some code that you could turn into test cases, or even code that you could send me and I could turn it into test cases. I'd just like to get some concrete code in front of me that demonstrates the potential problems; I think once we get out of the realm of the abstract I can move forward with extending the cases and that sort of thing. I just seem to have some sort of mental block about how to start this whole process.
I'm also still trying to wrap my brain around where $class
is going to come from. I guess some code along the lines of "ref @{[$signature->invocant]} || @{[$signature->invocant]}"
... that gets the right answer regardless of whether invocant is $self
or $class
. But then what about when there is no invocant? People can use types in func
signatures too ... right?
Would love to get moving on this so mst and ether can stop stalking me on the internets. :-)
Yup, I can write up some test cases. I have a clear idea how to do this.
$class
we can get from caller()
by looking up the stack at compile time. I worked this out while trying to fix defaults, Method::Signatures::Signature needs the line number the signature started on.
my $level = 0;
while( my @caller = caller($level) ) {
last unless @caller;
$level++;
next if $caller[0] =~ /^(Devel::Declare|Method::Signatures)/;
return $caller[2];
}
croak "Couldn't determine what line this signature is on.";
I'm going to be totally offline for over two weeks. I'll come back to this then.
It's all in the title... Type::Tiny is great, fast, light, minimalist, and works well with Moo. Can it be supported by M::S ? thanks