Tag Query v0.0.4

[davidanthoff]

No description provided.

[tkelman]

Extending base methods on base types like this

import Base.length
function length{T<:AbstractString}(s::Nullable{T})
   ...
end

is very much not recommended, as that changes the behavior of unrelated code.

[davidanthoff]

There was a suggestion here that for the 0.5 release cycle lifted versions of functions should live in packages, so that is what I'm doing here. I have no intention of keeping any of these in the package, as soon as these things have been added to base I'll remove my versions.

[tkelman]

It would be a better idea to centralize them then, rather than having each package have a slightly different set of them. Since you depend on NullableArrays, perhaps propose that these be added there for now, alongside JuliaStats/NullableArrays.jl#141 (comment) and other likely-temporary stopgaps until a more general solution to the lifting issue is worked out? This kind of transformation could also be done by a use-site macro rather than modifying shared global state of Base method tables.

[nalimilan]

FWIW, @davidagold plans to move lifted operators from NullableArrays to a new NullableOps package. Though lifting functions beyond standard operators sounds like a slippery slope to me: where are we going to stop. Better implement automated lifting semantics inside query macros.

[davidagold]

I do plan to do this, eventually. There's no timeline yet, but watch JuliaStats/NullableArrays.jl#148.

[davidanthoff]

Great news about the NullableOps package, that certainly seems the right way to go!

I think the best way forward for now is to just tag Query for now because a) I wouldn't want to create extra work and put things into NullableArrays if that is actually not where things should be in a few weeks, b) it sounds like NullableOps might take a while until it is there and I would very much like to press forward with Query versions and not wait for that. As I said, I'll gladly remove all of these lifting things from Query once there are better places for that in base or other packages, but in the meantime I would like to just continue releasing Query versions so that I can get feedback on the design etc.

This kind of transformation could also be done by a use-site macro
Better implement automated lifting semantics inside query macros.

That really doesn't square with the philosophy and design of Query, so I won't be going down that road.

[nalimilan]

That really doesn't square with the philosophy and design of Query, so I won't be going down that road.

Yet I don't see Julia Base providing a length method for Nullable{String} which would return the length of the wrapped string, and keeping it in a package in the long term would be confusing for users ("type piracy"). So really you/we will have to find a solution.

[davidanthoff]

I don't know what the best long term strategy would be. It is not clear to me that Base shouldn't provide something like that, but I think there is no need right now to figure this out. I think the best way forward on these things at this point is to experiment with different solutions. Yes, that means a little bit of chaos in the short term, but that is how we will learn and figure out which ideas are good and which aren't. If we only tag versions when we have made final decisions on all of these things we will just slow down things to a halt and end up with long discussions instead of code that can be tested and evaluated by a broader user group, and I don't think that is the right choice at this stage of the data handling ecosystem.

On the Query design point: as a general rule of thumb I don't want to give any function different semantics when used inside a @query statement. I think a system where functions start to behave differently depending on the context where they are called is a bad design choice, so I'm staying clear of that. Happy to discuss that point further, but an issue in the Query repo would be more appropriate than this PR here, where I really just hope to get a new patch version of the package tagged...

[tkelman]

Changing the behavior of totally unrelated code depending on whether or not your package has been imported can have really difficult-to-debug and undesirable consequences. That's a much worse way to make functions start behave differently than using a macro transformation. The former changes behavior globally, the latter is local.

[davidanthoff]

Yes, I agree, but Stefan had suggested that we do it that way until things are sorted out in Base. Note also that none of these changes the behavior of something in base, i.e. non of these override a base method, they only add new methods (unless you rely on length(Nullable("test")) throwing an error, which seems unlikely). And let me just repeat: I do not intend to have these things around in Query for a long time, this is a temporary thing until these things are sorted out either in other packages or in Base.

[tkelman]

Some of the other packages where lifting is an ongoing design issue may want to use reflection and the existence or non-existence of methods in Base to decide on their behavior.

[davidanthoff]

@tkelman None does, as far as I know. If you apply your criteria for tagging versions consistently you should never have tagged NullableArrays, and you should also not tag NullableOps once it is ready. It also means that @StefanKarpinski suggestions to experiment with these things in packages until they can be brought into Base is not practical because we then can't tag any of these packages.

I certainly understand and agree with your point in general. But I think if you make that a criteria for tagging versions in this Nullable space, you are slowing down the few folks that are actually trying to make progress here. In my opinion that is the last thing we need right now.

Having said that, I don't want to spend my time discussing this, I want to improve my package. If the only way to get a new version tagged is to remove this length method, I'll remove it. We'll have one less attempt to solve the Nullable problem, which I think is harmful for the wider problem space, but it is not my focus right now.

[tkelman]

Would rather have these method extensions centralized as I said above, otherwise it's a mess of inconsistent sets of methods that will conflict with one another. This is fine, if it's temporary.

[davidanthoff]

@tkelman Thanks, appreciated! I'll definitely try to move these out soonish.

[nalimilan]

My point was certainly not to say that the package shouldn't be tagged as-is, but rather that we should consider these issues seriously sooner than later. And indeed I know this is what we're all doing at the moment.