Fork me on GitHub
#clojure-spec
<
2019-10-02
>
Jakub Holý11:10:03

@alexmiller I have a tiny improvement to gensub to provide a meaningful error when such-that fails, is it meaningful for send a patch to https://clojure.atlassian.net/browse/CLJ-2097? Essentially (gen/such-that #(valid? spec %) g 100) -> (gen/such-that #(valid? spec %) g {:max-tries 100, :ex-fn (fn [{:keys [max-tries]}] (ex-info (str "Couldn't satisfy such-that predicate after " max-tries " tries.") {:path path}))}) (Since path is the only thing that looks meaningful, contrary to test.check's gen, pred and spec's spec, form) I have updated the issue, let me know there whether to send a patch. Thank you!

alexmiller12:10:06

test.check 0.10.0 has some new hooks for this case too

alexmiller12:10:27

if you have a patch, go for it, but I'm not sure whether we're going to go back and patch anything on spec.alpha. this is tbd, but my current thought is that once we get close to a spec 2 release I would reassess all the spec tickets (I know some have been fixed) and try to do a clean-up wave. this will require re-working any of the patches as I'd expect none of them to apply to spec 2.

👍 4
Jakub Holý15:10:11

Thanks, @alexmiller! Question 2: I discovered that it is even more useful to include an example of why the spec failed on sample data - knowing which spec failed to match is good but knowing why is even better. In my case the spec failed was something like ::person because of invalid :person :address :zip. So, to troubleshoot my issue, I include this in the thrown error:

{:max  max-tries
 :path path
 :sample-explain (->> (first (gen/sample g 1))
                      (explain-data spec)
                      :clojure.spec.alpha/problems)}
- which was extremely useful for me but not sure whether it would be an appropriate general solution.

kenny15:10:34

@holyjak YES. The failing args and ret should be easily shown in the error message. I don't think that particular approach will work because there's no guarantee you'll hit the failing case with one sample but I like the idea.

alexmiller15:10:30

showing an example which isn't the one that actually failed seems more confusing to me

kenny15:10:34

Yes - it needs to be the failed one.

Jakub Holý15:10:44

ALL the examples fail

kenny15:10:51

We have internal code that does this already but it's a hack on gensub.

Jakub Holý15:10:53

Well, in theory you are right. What we call is `(gen/such-that #(valid? spec %) g {:max-tries 1000 ...}) - none of the 1000 generated samples failed. What is the chance that the one we generate in the error does not fail?

kenny15:10:56

Depends on how you're controlling the size.

Jakub Holý15:10:08

What do you mean?

Jakub Holý15:10:29

Of course I could add a check and include :sample-explain only if it really fails. Would that be satisfactory? If not - what is a good way of capturing some generated, failing data?

kenny15:10:43

IIRC, gen/sample does not use all 200 size values.

kenny15:10:11

Using quick-check or st/check will.

Jakub Holý15:10:55

I am not sure how such-that invokes the generator but I would assume that it does so in a way very similar to sample?

Jakub Holý15:10:06

Anyway, what do you think about the "best effort" approach - generate a sample, if it fails the spec, include :sample-explain in the error. I think that would work in 99.9% cases?

kenny15:10:56

Anyway, the piece I find valuable is being able to easily see why a check failed, whether it be by generator failure or check failure. When a generator fails, I would like to see the failed samples. This usually lets me figure out which predicate is causing the issue pretty quick. In the case the check fails, I'd like to see the args and the ret.

kenny15:10:08

You should know which samples failed while generating @holyjak, no need to generate new ones.

Jakub Holý15:10:02

Well, I do not generate anything, it is such-that that does it - and in its failure-handling function I have no idea what values were actually tried. Or? BTW here is the code https://gist.github.com/holyjak/8cadc0d939c8e637ef6bf75b070d28b4#file-clojure-spec-alpha-clj-L17

kenny15:10:06

Our hack (not acceptable for a patch but let's us debug generators without going crazy) is to store invalid values in an atom and display that in the error message.

kenny15:10:33

So we just change the such-that predicate to do that.

👍 4
Jakub Holý15:10:09

Smart! The question is: What approach would be acceptible for a patch (and useful for the users!)?

kenny15:10:13

I don't know. It definitely feels too hacky for a patch. It may be the only way to do it though. When I was looking at this, I don't think I saw any test.check hook to grab the failed values.

kenny15:10:26

Just speculating but I imagine it'd be somewhat easy to add that hook to test.check if it isn't already there and is the proper solution.

kenny15:10:54

I would definitely be in favor of adding some sort of patch to spec1 alpha given we don't know when spec2 is coming. Debugging generators is super painful as it is.

gfredericks15:10:58

Check the such-that docstring in the latest version

kenny15:10:52

Doesn't appear to have anything that stores failed samples.

gfredericks15:10:20

Would you want all of them? Just the latest?

gfredericks15:10:38

Having the generator means it should be easy to generate more

kenny15:10:43

Probably all.

kenny15:10:26

If I have all, I can easily only have the latest if I want.

Jakub Holý15:10:34

I believe a single value is enough (preferably the 1st as it is the simplest one)?

gfredericks15:10:10

Keeping all could be a GC issue

Jakub Holý15:10:18

> Having the generator means it should be easy to generate more That is what I do in my solution (generate a new value using sample) but it has been argued that using one of the actually failed would be better.

kenny15:10:46

I guess I only ever work with one of the failed samples anyway.

Jakub Holý15:10:13

@gfredericks As I explained above, this popped up when working with specs - a custom generator generated value that did not conform to the spec. I can get the name of the spec but want to see why the custom gen failed to provide anything valid. There, having a sample value and running s/explain-data on it is sufficient and very useful.

kenny15:10:23

Looks easy to add in such-that-helper.

Jakub Holý15:10:47

I am creating an issue on test.check to pass a sample failed value to ex-fn. Stop me if I should not 🙂

gfredericks15:10:04

You shouldn't not

👍 4
Jakub Holý15:10:04

Conclusion: I create a patch for Spec to include the spec name (or rather path) in the such-that failures and we wait for test.check to expose a sample failed value to the :ex-fn before adding explain-data of it.

Jakub Holý15:10:14

@alexmiller is there any point in providing also a similar patch for Spec 2 or is it too much in flux?

alexmiller15:10:41

I'm not going to do the sample thing, so not interested in a patch for that

Jakub Holý15:10:02

no, not that, just including path

alexmiller15:10:06

that's fine

👍 4
alexmiller15:10:14

I don't think there's any point for spec 2

👍 4
alexmiller15:10:39

one possible outcome is that we repackage everything in spec 2 to remove alpha designation, which will (again) break any patches

alexmiller15:10:03

I think in test.check, while generating in such-that, it would be possible to just retain the first or last gen'ed every time (before the pred check) and report that if you hit the retry limit. that way you're reporting an example that was actually tried.

gfredericks15:10:59

An example generated after the fact is just as meaningful, philosophically

alexmiller15:10:01

if only a percentage of them fail, and you happen to generate a sample that doesn't fail, that seems strongly less useful than one of the actual examples that didn't pass the predicate

alexmiller15:10:07

so I'ma disagree with you on that

gfredericks15:10:22

I was assuming you'd filter on failure 😛

alexmiller15:10:43

well you failed to mention that :)

alexmiller15:10:48

but that would be fine

gfredericks15:10:51

Which, ironically, could fail :face_with_rolling_eyes: though presumably is unlikely to

gfredericks15:10:23

But that might be a good enough reason not to do it

gfredericks15:10:58

Adding the size to the failure data would assist with that though

Jakub Holý15:10:31

@gfredericks Will you consider adding a sample failed value to the arguments of such-that's :ex-fn or some alternative improvement?

gfredericks16:10:41

@holyjak I'm not going to be doing active work on test.check for the foreseeable future, so that'll be up to somebody else; but a jira ticket is definitely the right way to make sure it gets looked at

Jakub Holý16:10:13

Hm, I was about to create it but you told me I should not https://clojurians.slack.com/archives/C1B1BB2Q3/p1570030324189400 I guess it was just misunderstanding.

Jakub Holý16:10:41

I'd be happy to provide a patch to test.check - and even happier to get any pointers regarding the best solution.

gfredericks16:10:07

No I meant you should create the ticket

gfredericks16:10:31

Sorry, I was being cute with English double negatives, it was probably a bad idea

Jakub Holý16:10:49

I see 🙂 My bad, I should have read more carefully. Also, I believed that English normally doesn't permit double negatives. Always learning 🙂

Jakub Holý16:10:33

Any pointers regarding the best way to implement this? Is failed-value a good key name to pass to :ex-fn?

Jakub Holý16:10:03

FYI I have created https://clojure.atlassian.net/browse/TCHECK-156 Provide sample failed value to such-that's ex-fn for better error messages

👍 4
Jakub Holý16:10:17

FYI I have sumbitted a patch for ☝️

noprompt23:10:29

What is the proper way to lawfully copy/paste source code from core.specs.alpha?

noprompt23:10:28

What I’d like to do is copy ::defn-args and its dependencies into my own project.

noprompt23:10:05

Do I just put the copy right information above the code I want to copy?

noprompt23:10:55

Also, my project is MIT licensed, so I’m not sure how all that works.

noprompt23:10:34

> What are my obligations if I copy source code obtained from http://Eclipse.org and licensed under the Eclipse Public License and include it in my product that I then distribute? > Source code licensed under the EPL may only be redistributed under the EPL.

noprompt23:10:04

I’m guessing I need to handwrite my own version of spec.

seancorfield23:10:25

@noprompt I'm curious as to how you're depending on specs from that namespace since they are intended to support clojure.core macros?

seancorfield23:10:03

(and, yes, you're going to be on some pretty shaky ground if you copy'n'paste that code into your system and then distribute it)

noprompt23:10:56

@seancorfield Imagine you want to write a macro that has the form as defn.

noprompt23:10:32

And imagine that I want to use conform to parse ::core.specs/defn-args.

noprompt23:10:52

Thats my situation.

seancorfield23:10:01

Ah, so your code was relying on the internal names used in the those specs?

noprompt23:10:18

I’m noticing you’re using the word “internal”.

seancorfield23:10:34

Only two top-level specs changed their names.

seancorfield23:10:04

If you conform, you have to rely on the implementation of the spec.

noprompt23:10:18

Yeah but specs are globally public…

seancorfield23:10:48

We've used alpha versions of Clojure libs in production for over eight years and sometimes Cognitect change stuff while it's alpha and break your code. That's the risk of using alpha libs.

noprompt23:10:11

I hope you can appreciate the irony in this.

noprompt23:10:44

To me its a bit surprising given the critique of SemVer, etc.

noprompt23:10:58

So suddenly calling it “alpha” makes it okay to make breaking changes?

noprompt23:10:24

::defn-args2?

seancorfield23:10:28

They've been very clear that alpha means "can change/break". Things are only guaranteed additive/fixative once the alpha label goes away.

noprompt23:10:48

Its not on the README.

noprompt23:10:56

Where were they clear?

seancorfield23:10:25

They've said it repeatedly publicly. Pretty sure even Rich has mentioned that in talks...?

jaihindhreddy06:10:18

Rich says it in the same talk where he contends SemVer is broken. Search for alpha in https://github.com/matthiasn/talk-transcripts/blob/master/Hickey_Rich/Spec_ulation.md Funny thing though, Rich also says something like:

But, that is not to say just leave your thing 0.0.967. At a certain point, you are going to have users, and whether you change it to 1.0 or not, they are going to be depending on your stuff.
And that is where spec is going right now IMHO. But his Maybe Not talk and Alex Miller's blog eased my fears of spec remaining in this state forever.

noprompt23:10:42

Like on Twitter?

noprompt23:10:20

I rarely, if ever, watch talks.

noprompt23:10:45

The README and the source code are far more public.

noprompt23:10:52

The disclaimer should be there.

taylor23:10:22

what would it say

noprompt23:10:30

What Sean said.

noprompt23:10:00

How about > Alpha means this library “can change/break” do not rely on it.

noprompt23:10:53

Because I obviously misunderstood the situation.

seancorfield23:10:54

I'm sorry you've missed it being discussed on the mailing list and in Rich's talks and here on Slack (where it has come up several times in several situations).

noprompt23:10:36

I just want answers to my questions.

noprompt23:10:06

Can/How do I copy the source code?

seancorfield23:10:18

Not legally into anything you are distributing under an incompatible license.

seancorfield23:10:17

If you want to use EPL code in your OSS project, consider changing your license to EPL so it's compatible.

noprompt23:10:21

Okay, that’s what I gathered by reading the EPL website.

noprompt23:10:05

Its fine, I can write the specs myself in this case. I was just hoping I could take a short cut.

seancorfield23:10:34

Given the day I've had working on our Spec 2 branch, I do sympathize (even if it may not sound like it).

noprompt23:10:26

Honestly, it didn’t feel that way but you saying that makes me feel like you do.

noprompt23:10:50

I just want to get my users and teammates out of a broken state quickly.

noprompt23:10:58

I’ve given up on trying to supply feedback or contribute to anything in the clojure repo due to conversations that go the way they do regarding these kinds of frustrations.

seancorfield23:10:18

Yeah, I can detect a fair amount of frustration/anger...

noprompt23:10:25

Transparency would be nice on these topics; one can’t be expected to keep up with every feed of data.

noprompt23:10:58

The repo and source code is about as public and as inclusive as any place to explain semantics.

noprompt23:10:40

I’ve made the effort in Meander to call out the versioning semantics I employ so there is no misunderstanding.

seancorfield23:10:45

When Rich introduced spec in 2016, he was pretty clear that it was alpha and subject to change (I just checked the transcript of one of his talks from back then). He also called out alphas as "potentially breaking" in his Spec-ulation talk (which is where his main criticism of SemVer was made.

seancorfield23:10:53

SemVer itself is broken tho'. While you're in alpha, you can make (breaking) changes. Once you have a non-alpha, you should only make accretive/fixative changes. SemVer doesn't help with that.

👍 4
noprompt23:10:59

That makes this assumption that people will be exposed to those things or, if you have my memory, remember them.

noprompt23:10:47

Just slap a phrase on the README that says, upfront, what the version semantics are instead of relying on them being implicitly understood.

noprompt23:10:11

What I took from Rich was if I’m going to make a breaking change, I make a new namespace, artifact, etc.

noprompt23:10:21

And that works fantastic.

seancorfield23:10:24

If you're not in alpha, yeah.

noprompt23:10:34

Its a greek letter.

seancorfield23:10:45

It was an important caveat in his talk 🙂

noprompt23:10:55

Rich doesn’t get to decide what that means.

noprompt23:10:03

He can’t have it both ways.

seancorfield23:10:05

(and it's been the practice in Clojure overall for years)

noprompt23:10:16

Saying that “alpha” can break is just weird thing.

noprompt23:10:29

None of these versions can break but this one can.

noprompt23:10:01

But, hey, if the semantics were with the source, fine, whatever, those are Rich’s rules. Cool.

noprompt23:10:43

My point is they’re not transparent.