search

brownplt / pyret-lang

The Pyret language.
Other
1.07k stars 109 forks source link

cannot use images with types #1237

Closed shriram closed 6 years ago

shriram commented 6 years ago 
include image
rectangle(1, 2, "solid", "blue")

produces the error

image

sorawee commented 6 years ago

https://github.com/brownplt/code.pyret.org/blob/horizon/src/web/js/trove/image.js is needed to be changed.

blerner commented 6 years ago

Yes.....and we should decide for really-real this time, whether we're going to switch from a stringly-typed API to a strongly-typed one.

schanzer commented 6 years ago

I'm in favor of the switching, as long as we have a compatibility story. But either way, we should include the proper types in both libraries.

mkolosick commented 6 years ago

What should I do about ImageColor in, for example, the type of circle?

schanzer commented 6 years ago

In the old API? Those functions consume strings. If those don’t map to a color, it’s a runtime error

blerner commented 6 years ago

Right. The question is, concretely, how and when do we transition to the new API with Color arguments. I'd like to avoid Racket's fate of having "images" and "images2", and would rather have "images" be the better typed module. The only sticking point is how that impacts Bootstrap and its pedagogy, and I know you're concerned to avoid any unnecessary concepts whenever possible. Are Colors unnecessary, or helpful? How would you like to see the transition between apis happen?

schanzer commented 6 years ago

Something I’ve learned from data science is that you want to keep the number of types to “do the basics“ as small as possible. My gut sense is that we could provide the old API through the Pyret equivalent of a teachpack, and then introduce the real API partway through the class for students and teachers who are interested.

@Emmay , would you have a problem with requiring all reactive programs to have an include line at the top?

Emmay commented 6 years ago

All reactive programs already use two include lines: include image, and include reactors. Would this change the image library at all, or change the need for including that library? If the library is only changing under the hood, there won't be any change for reactive programs.

On Sun, Nov 5, 2017 at 12:26 PM Emmanuel Schanzer notifications@github.com wrote:

Something I’ve learned from data science is that you want to keep the number of types to “do the basics“ as small as possible. My gut sense is that we could provide the old API through the Pyret equivalent of a teachpack, and then introduce the real API partway through the class for students and teachers who are interested.

@Emmay https://github.com/emmay , would you have a problem with requiring all reactive programs to have an include line at the top?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brownplt/pyret-lang/issues/1237#issuecomment-341993991, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1MdIXMn4TW8EXvRLBWHIL4hVtO94z8ks5szf3agaJpZM4QNdXl .

blerner commented 6 years ago

I'm suggesting that it would change the image library, and we could provide a "simple-image" library that continues to provide the use-strings-as-color-names API that we currently have.

schanzer commented 6 years ago

@shriram , Now that the migration question is settled, and we have a library that addresses the issue, is this OK to close?