code.pyret.org
code.pyret.org copied to clipboard
brownplt
name-to-color's type is not accurate
The doc states that name-to-color's type is String -> Color. However:
> name-to-color("")
false
returns a Boolean.
The type is right; the code is wrong. More accurately, it should return an Option<Color>, if we're going to be pedantic about erroneous color names.
But I think the issue is that we want to avoid Option which is an overhead for students...
Then you're stuck with throwing an uncatchable fatal error if the color name is bogus. Pick your poison...
I think the existing behavior is absolutely critical for Bootstrap. And given the increased focus on Bootstrap's userbase, can we be happy with the status quo and close this issue?
If anyone else complains, we can revisit in a new issue.
No, I'm not happy with that status quo. The new image library, whose APIs we've discussed at length, now has name-to-color that returns an Option<Color>, and a color-named that returns a Color or throws an error. Designing a function as part of the standard image library (either stringly-typed or structurally-typed versions) that is intrinsically untypeable in any useful way seems like a horrible idea. You can still build your own teachpacks, though, that wrap name-to-color and do whatever you want with that Option, but I don't want the standard library to be broken like this.
Given our multiple target audiences, why shouldn't the "standard" library contain both? I feel this is analogous to the two-valued vs three-valued equality predicates, and we made a good decision there. I think we should carry that design idea forward.
If you don't want to mess with datatypes, you use the one with the overly generous type and deal with dynamic crashes. This is fine for beginners. If you want to program defensively, you program with the more refined type, and can handle problems as they arise — better for programmers with the tools under their belt and who have need for that robustness. Implementing the former in terms of the latter is of course quite trivial, and should be done as a matter of policy to keep the error behavior strictly analogous.
Having a consistent naming of course matters. For equality, we took the view (which I think is right) that the shorter, simpler name should be the beginner-friendly version, while the more robust version has a longer name.
In cs019, we actually discuss this design (in the context of equality), and I think it's very enlightening to at least some students.
That's exactly why we created color-named, which had a shorter name and dynamically crashes if you give it garbage inputs. I don't think we want Pyret to ever have functions that have Racket-like pseudounion types of "Something or False", and I don't want our standard library to have a function whose only possible type is String -> Any.
Okay, so do we have two functions here:
f1 :: String -> Color [raises exception]
f2 :: String -> Option<Color>
If so, we are set, and the OP should be considered a code, not type/documentation error.
I agree that we don't really want the Rackety false response. That then impacts our type system but also our notion of truth. We've held the line on not being a truthy/falsy language, and this violates that.
From pyret-horizon, right now:
The same output occurs if you include image-typed instead.
Can we choose a more consistent naming, with a suffix to distinguish them? No need to get poetic — how about color-named and color-named-opt or name-to-color and name-to-color-opt?
We kept name-to-color because it was a legacy name and you and Emanuel insisted we needed to keep it for back-compat reasons; we added color-named because it was shorter. We can alias name-to-color to color-named-opt if you want, but I'm not super keen on adding yet more aliases and synonyms.
Sure, but I would rather make a consistent named pair the primary interface and keep the legacy name alias around for a while but deprecated in the docs. Update the curriculum. In two years, we can remove the legacy name.
Interestingly, going back to find this history, I see we had a thread in Aug 16 where Joe said:
"Naming s2c well seems important; right now it's present as
name-to-color, but it returns false rather than throwing an error, which
should be considered a bug."
Shriram