skade has quit [Quit: Computer has gone to sleep.]
dkubb has joined #rom-rb
knowtheory has joined #rom-rb
knowtheory has quit [Client Quit]
knowtheory has joined #rom-rb
knowtheory has quit [Read error: Connection reset by peer]
knowtheo1y has joined #rom-rb
knowtheo1y has quit [Ping timeout: 265 seconds]
snusnu has joined #rom-rb
<elskwid>
dkubb: ping
knowtheory has joined #rom-rb
<dkubb>
elskwid: pong
<dkubb>
elskwid: how's that type inference bug going? If you can repro it in something stand-alone i could help to determine if it's a bug in axiom-types or virtus
<elskwid>
dkubb: It's not as much a bug as it is a very complicated problem.
<elskwid>
dkubb: It centers around the use of `primitive` and the type lookup that Virtus does internally
<elskwid>
dkubb: And for a specific issue it is coupled with the complexity of the Boolean type.
<elskwid>
*sigh*
<dkubb>
probably due to how it has to work with TrueClass and FalseClass
<elskwid>
dkubb: Yessir
<dkubb>
in axiom-types I dealt with the same problem. I got rid of the primitive specific check and use an inclusion test now
<elskwid>
dkubb: When solnic and I were talking earlier today he was complaining that the type lookup in Virtus is his least favorite part and that it is difficult to get right. Looks like there is a type cache internal to Virtus that doesn't play well with what we called "built-in primitives"
<elskwid>
and I can see that it is a complicated interface ..
<dkubb>
where in virtus is type lookups used?
<dkubb>
is it only in the attribute() dsl?
<dkubb>
what I was wondering is if it's only in the attribute() dsl stuff, then perhaps the code could be changed to use the Virtus::Attribute if it was provided as an argument, otherwise falling back to asking Axiom::Types.infer to return the Axiom::Type::Type subclas
<dkubb>
then virtus would have a simple map of primitive -> Axiom::Types::Type
<elskwid>
dkubb: TypeLookup is mixed in to Attribute and `determine_type` is used as the builder sets things up.
<dkubb>
the first lookup would do the work, and subsequent lookups would just use the hash
<dkubb>
whoops, my statement was wrong
<dkubb>
I meant there would be a map of Axiom::Types::Type classes to Virtus::Attribute classes
<elskwid>
I have such a weak grasp on the type look stuff anyway. I spent the day just threading through Virtus, Coercible, and Axiom::Types to get my head in teh game.
<elskwid>
*the
<dkubb>
yeah it's a bit mind bending
<dkubb>
I'm not exactly happy with Axiom::Type's own inference, but it was the best I could come up with at the time. I have a few ideas I may try as time permits though
<dkubb>
if there are actual bugs in the implementation that would probably get me looking sooner rather than later
<dkubb>
type lookups is hard because on the surface it looks like all you need to do is map primitive classes to some kind of object
<dkubb>
but when you throw in the fact that you can specify Set[String], now a few specific instance types need to be factored in
<elskwid>
Right.
<dkubb>
axiom-types takes the simple, if somewhat naive, approach of searching through the types for the first match and returning that, and caching it for the next lookup. it works well if there is one unabiguous mapping of a primitive class to a type
<dkubb>
it gets tricky when there are multiple matches
<elskwid>
I think Virtus is doing that internally as well
<dkubb>
but the way new types are added to the list always puts them in the same order, and appends them to the end
<dkubb>
fwiw, I wrote my inference after virtus'
<elskwid>
Virtus suffers from a few cross-cutting concerns that make it confusing: coercers, and types - seeing as they are both backed by libs but the terms are used internally for classes that wrap the functionality.
<elskwid>
it gets difficult to know from whence a coercer came
<elskwid>
heh
<dkubb>
yeah, it's hard to test it
<elskwid>
There have been a couple of issues reported that show people are trying to use the Virtus::Attribute interface to build custom attributes
<elskwid>
and they expect `primitive` to give them the base type upon which to build the attribute
<elskwid>
but because the type lookup is wonky it doesn't work well when you use Date, or Boolean - and for that matter, probably String, etc.
<elskwid>
It turns out that the internal Virtus::Attribute::Boolean is using the exact same premise and it is broken. The coercion just doesn't work.
<elskwid>
Strict mode doesn't work either
<dkubb>
I can think of one change to virtus that might help
<elskwid>
It maps the type and we get BasicObject back from Axiom::Types and the coercion is just a simple, "return what I was given"
<elskwid>
well, I'm all for experimenting
<dkubb>
right now each Attribute has a primitive class, and some code *asks* the attribute for it's primitive and uses that to compare against some object
<elskwid>
The way I see it, I want to get the contract for 1.x working and solnic can take a look at what we did later. ;-)
<elskwid>
yes
<dkubb>
what I would suggest is not doing that and instead have a #primitive?(object) interface where you pass the object into the Attribute and it can do it's own logic
<dkubb>
the base implementation can be a simple equality test like what happens now
<dkubb>
but for boolean it can do a bit more
<dkubb>
but the main thing is the logic can be encapsulated in the Attribute, and it can handle other more complex scenarios besides this
<dkubb>
in axiom-types I used something similar to this at first
<dkubb>
I eventually refactored it so that you pass in an object, and it will return the correct type (either itself, another type, or something it builds to handle the object)
<dkubb>
or nil if it can'd handle it at all
<elskwid>
I've seen that in axiom-types
<dkubb>
in which case the code just goes to the next type and tells it what the object is and for it to "construct" a type if it can
<dkubb>
I like this better because it allows me to do whatever I like. it's more inline with tell don't ask too
<elskwid>
yes, I've spent a while looking at this
<dkubb>
I think it's superior to even #primitive?(object)
<dkubb>
but if you don't know the reason behind it, it can be confusing
<dkubb>
it's precisely for reasons like Boolean that I went down this route
<elskwid>
I'll say this, for the most part, all of this code is easy to understand, it just requires a LARGE context to be loaded first
<dkubb>
yeah
<elskwid>
and with the reliance on external libs you need to have your head around that too
<dkubb>
you're mind if fresh, so I appreciate comments and critisims or even comments about what you found difficult to understand
<dkubb>
what I would much rather like to see is all inference removed from virtus
<dkubb>
and for it to have a simple mapping of axiom types to virtus attributes
<elskwid>
I'm focused on the user problems at the moment and I'm keen to get them "solved"
<dkubb>
but I don't know what virtus needs on my end to make that happen
<elskwid>
it looks like that mapping was started
<elskwid>
solnic mentioned that he couldn't take the type lookup complete out for some reason
<elskwid>
(I should probably find out that reason)
<dkubb>
yeah, I would suggest looking at refactoring towards a #primitive?(object) or the #infer interfaces
<dkubb>
yeah I heard that too, but I don't think I understood exactly why
<dkubb>
if you can find out from him and then explain it to me maybe we can figure something out
<elskwid>
What I may do is have you take a look at a stop-gap solution to this primitive problem
<dkubb>
I could just ask him directly but it's probably good for you to have an understanding.. you'll also gain good understanding by trying to communicate it with me after :)
<elskwid>
Yes, that will help me a great deal. He's out for the count for the next few days I expect.
<dkubb>
elskwid: I'd be happy to review any code that you write, but I probably won't be able to do any direct virtus maintenance myself
<elskwid>
dkubb: Nor would I ask you to. I would just appreciate a second set of eyes with some amount of experience with (a) your style, (b) the project, (c) solnic's preferences.
<dkubb>
no worries, I'm totally able to provide that
<elskwid>
dkubb: Outside of that, I'm on the hook for this one.
<elskwid>
and I'm happy to do it.
<elskwid>
It's a very good challenge for me. Thank you!
<dkubb>
I just need to make sure I try to focus on getting the axiom stuff down since only a couple of us have dived down into it enough to do maintenance on it, while virtus has a much larger community around it ;)
<dkubb>
I have no problem helping you with whatever you need or pointing you in the right direction
<dkubb>
we're about to start doing some stuff on the sql generation side of things, which I think mbj is leading and I will be assisting
<dkubb>
were you getting back something to handle BasicObject instead of Boolean?
<elskwid>
dkubb: The coercer looks like this: #<Virtus::Attribute::Coercer:0x007fab8b061df8 @type=Axiom::Types::Boolean, @primitive=BasicObject ...
<elskwid>
and the check if the coercion worked uses the primitive
<elskwid>
which always passes
<elskwid>
even if I'm getting a string back or whatever, which means the coercion is actually failing the user expectations but in strict mode no exception is raised
<dkubb>
ok, so the axiom type is returning correctly, that's good at least
<dkubb>
are you able to write a failing spec for this and mark it as pending?
<elskwid>
Now that I know that it's actually failing I should be able to do that.
<elskwid>
It took me a long time to figure out what was happening.
<dkubb>
what I'd suggest is creating a branch and then make a PR out of it, then add the failing spec marked as pending
<dkubb>
use the pending do... end syntax to wrap your failing spec so that it still gets executed
<elskwid>
Oh yeah, there will be a PR tomorrow at some point.
<dkubb>
then when you actually get it passing, rspec will tell you so you can remove the pending block
<elskwid>
okay, that's rspec goodness, cool
<dkubb>
then inside that branch I'd probably do some WIP commits to see if you can get something working
<elskwid>
yep
<elskwid>
there will be noodling, oh yes, there will be noodling
<dkubb>
that'd be my recommendation.. I sometimes use the PRs as a way of exposing my thought process to the world even if I don't know the solution
<elskwid>
I'm going to let the day's efforts soak in a bit and tackle this tomorrow
<elskwid>
dkubb: very good recommendation
<dkubb>
in virtus where does the inference start?
<elskwid>
dkubb: Would you like a github link or just discuss the classes?
<dkubb>
yeah, I see this but I wonder why it needs to be so complex
<dkubb>
in virtus does the code still map primitives to virtus attribute objects?
<elskwid>
solnic mentioned that people had come to rely on some aspect of this type lookup interface
<dkubb>
or does it simply map axiom types to attribute objects
<elskwid>
the latter I believe
<dkubb>
hmm
<elskwid>
all of the virtus type attribute objects were removed with the exception of Boolean
<dkubb>
anytime I see the word "primitive" in virtus code it would make me suspect
<elskwid>
agree
<dkubb>
ideally virtus should have no concept of primitive objects. it should know about axiom types, who themselves know about primitives
<elskwid>
After today I would say I enthusiastically agree
<dkubb>
it's quite possible axiom types has it's own inference bugs, but at least then it's self contained in a (relative to virtus) simple library that is probably easier to debug
<dkubb>
what you *could* do is have it so anywhere that a primitive is specified it delegates down to Axiom::Types.infer and returns a type which you use inside virtus
<dkubb>
so then the concepts of a primitive simply passes through but isn't handled anywhere in virtus
<dkubb>
this would probably allow the existing interface to continue as-is
<dkubb>
hmm. although I'm thinking about how this will work with custom attributes
<elskwid>
I can't be certain but I think that primitive is coming out of axiom-types and just stored
<elskwid>
the idea being you can override with the options or at the class level
<elskwid>
for those custom attributes ...
<dkubb>
I suppose virtus could first ask axiom types to infer something for a given primitive.. and if it doesn't find anything specific (meaning it returns Axiom::Types::Object) then it could create a type on the fly for the primitive and return it
<dkubb>
... if that makes any sense
<elskwid>
that sounds similar to what is happening already
<elskwid>
there is that first check with axiom-types
<elskwid>
which uses the primitive from the TypeDefinition
<dkubb>
except I *think* it then goes and asks all the custom attributes for their primitives
<dkubb>
I'm saying when the custom attribute is declared it should do the inference/type creation, then add the type to the Hash mapping it to itself
<elskwid>
yes, the determine_type_from_primitive asks the descendants
<elskwid>
ah
<dkubb>
my gut feeling is we should be able to rip out almost all of the inference logic
<dkubb>
solnic said he did run into one problem with complex types, but I'd love to hear more about that
<elskwid>
I plan to get that failing test in and then see what I can do
<dkubb>
sure
<elskwid>
I found myself going round and round with that bit today
<dkubb>
see if you can fix this specific issue, and if you can then we'll have better test coverage to support a larger refactoring
<elskwid>
and that is usually a sign
<elskwid>
yes, precisely
<dkubb>
maybe look towards the #primitive?(object) idea
<dkubb>
I bet it'll only result in a few lines of code change
<dkubb>
maybe 20-30
<dkubb>
you can find the place in the code that does the object == attribute.primitive test, and change it to attribute.primitive?(object)
<dkubb>
and then have the standard comparison (whatever it is) happen in the base class, and then override it in the boolean attribute
<dkubb>
I did *exactly* the same refactoring steps in axiom-types
<dkubb>
because I modelled my original code off virtus
<dkubb>
then I refactored to this, and then I refactored to what you see now
<elskwid>
It's interesting you mention these steps because I wanted to do the override in boolean but it isn't really in a clear place at the momeny
<elskwid>
*moment
<dkubb>
each step resulted in either a simplification of the logic or an increase in the power I had
<elskwid>
again, usually a sign
<dkubb>
yeah, I mean you can't really overide it in boolean
<dkubb>
because boolean isn't responsible for it
<dkubb>
the caller code that uses boolean is
<elskwid>
right
<dkubb>
and unless you want to put a special case in the caller code, you're better off moving it inside the object
<dkubb>
it's a sign of feature envy (the code smell)
<elskwid>
yep
<dkubb>
I bet if you ran reek on the specific bit of code that handles this now it'd flag feature envy
<dkubb>
maybe check the config/reek.yml to see if it's being ignored
<dkubb>
my experience is that reek is right far more often than it's wrong
<elskwid>
yeah
<elskwid>
dkubb: Thank you very much for taking the time to chat with me tonight. It has been very helpful.
<elskwid>
I'm going to go add a contributing file to virtus to explain the state of the project (bug-fix mode) and create an experimental branch where I can pull in people's feature suggestions to try them out.
<elskwid>
then I'm off to anothe project for the night.
<dkubb>
sweet
<dkubb>
elskwid: no worries, feel free to ping me with questions anytime
<dkubb>
if you can get the JSON into an enumerable, you could use https://github.com/solnic/coercible to coerce things into the correct datatype and then feed it into axiom for processing
<dkubb>
I'd like to add some coercion functions to axiom at some point though
<warmwaffles>
you and solnic have sweet libs
<dkubb>
thanks :)
<dkubb>
we're working on building up an ecosystem of small, simple libs
<dkubb>
mbj and snusnu too
<warmwaffles>
Ah didn't realize that virtus used coercible under the hood
<dkubb>
and others.. like elswid are helping with virtus
<dkubb>
yup, it was extracted from virtus
<dkubb>
this is how some of these libs start. we bake them in, and then realize the lib is doing too much or something besides it's core responsibility, so then we break it out
<dkubb>
adamantium and ice_nine were extracted from axiom for example
<warmwaffles>
though with the new syntax with virtus it feels wrong to do `include Virtus.model` I'd rather say something like `include Virtus::BasicModel` or some shit
<dkubb>
yeah, I'm on the fence about that
<warmwaffles>
meh, I'm just picky I guess
<dkubb>
I think there are modules you can use
<dkubb>
I think he went with that approach so that the module names could be changed
<dkubb>
or things could be broken up and moved around
<warmwaffles>
yea, and then when shit is finalized, it can be moved into a permanent residence?
<dkubb>
probably the weirdest thing about is is that it's using a module instance. that's not used very often in ruby.. I use it in equalizer and mbj uses it in anima, but it's still uncommon
<warmwaffles>
I went and built my own version of virtus to see how possible / complex the problem was and try to get a better understanding for things
<warmwaffles>
Coercion is the hardest of them all
<elskwid>
Hey @dkubb, what is the best channel to get you information? I have you on Twitter and IRC. Email? Certified letter?
<elskwid>
I have a gist here that isn't for public consumption. Would like your thoughts.
<dkubb>
heh
<dkubb>
elskwid: you can always email me at dan.kubb@gmail.com .. if it's really sensitive I have a published gpg key you can use
<elskwid>
Not that sensitive, just not totally public. Email is on the way.
<warmwaffles>
que fancy
<elskwid>
Thank you!
<dkubb>
warmwaffles: yeah, it starts off really simple, as soon as you start adding features things get complex
<warmwaffles>
almost too complex
<warmwaffles>
lol
<dkubb>
warmwaffles: I think virtus could probably be refactored to be simpler in a few areas though, but it's also possible I don't understand all the pieces well enough
<dkubb>
at a certain point you also have to ask "am I doing too much" .. even if virtus is delegating to a bunch of other libs, there still may come a time when it is doing too much
<warmwaffles>
I'm thinking I may coerce the data before I start doing anything with the relational data
<warmwaffles>
only because I know it's got to be done, and doing it at runtime for the relation is a little dumb
<elskwid>
dkubb: hear, hear. Part of the problem is the understanding of "why" which leads to the "what
<warmwaffles>
gotta go
warmwaffles is now known as warmwaffles|afk
<dkubb>
warmwaffles: yeah, at the moment that's what I'd do. coerce it to the matching types up-front will be easier. coercion will probably be added to axiom (mostly because it's part of the "D" specification).. and it will probably use solnic's lib
<dkubb>
kk
zekefast has quit [Quit: Leaving.]
<elskwid>
dkubb: I have to run but I wanted to leave you with this thought: what about a type registry that is backed by axiom-types? Something that abstracts this common problem away for custom types and still has a solid underpinning.
<elskwid>
^ floating around in my head this morning.
<elskwid>
I'll be back later.
<dkubb>
I think that's what I was proposing last night too
<dkubb>
k, ttyl
<elskwid>
dkubb: Yes. I'm wondering about axiom-type-registry
<dkubb>
if I alias .infer to .[] then you could lookup a type by doing: Axiom::Types[String] would return Axiom::Types::String, and Axiom::Types[TrueClass] would return Axiom::Types::Boolean
postmodern has quit [Quit: Leaving]
solnic has joined #rom-rb
<solnic>
dkubb: hey, I’m here for a second - just to clarify, virtus’ module build (.model, .module and .value_object) builds a module that carries *configuration* with it, that’s why we added that. I also like it bkz it results in shorter syntax. ok, I’m out. cheers
solnic has left #rom-rb [#rom-rb]
zekefast has joined #rom-rb
<elskwid>
dkubb: I got it. I'll look that over in a bit.
irclogger__ has joined #rom-rb
warmwaffles|afk has quit [Ping timeout: 240 seconds]
irclogger__ has quit [Remote host closed the connection]