-
Notifications
You must be signed in to change notification settings - Fork 302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: unify float
and int
as simply number
#253
Comments
Original reply by @rudolph9 in cuelang/cue#253 (comment) |
Original reply by @mpvl in cuelang/cue#253 (comment) This does simplify things considerably, which is good. For good measure, let me bring up some clarifications and concerns:
Binary floating points (what CUE uses) allow integer values with decimals. (e.g. This also means that, say One concern is that this may make configurations somewhat unstable: for instance, a configuration may have a field Another concern is that non-integer values may unify with An alternative is to remove the ability to require a field to be of type float. The predeclared identifier But overall, this is definitely worth considering. It simplifies things considerably at various places. NCL, a Google-internal configuration language does something similar and this seems to work fairly well. The main drawback there is the inability to constrain a number to |
Original reply by @mpvl in cuelang/cue#253 (comment) I can see the following options:
Option 3 would have most of the protections of the current CUE implementation, while being simpler, while Option 4 would be the simplest and offers slightly better JSON compatibility (allowing 0.0 to be assigned to integers). The latter could be addressed with JSON interpretation, but this may be confusing as CUE is a superset of JSON. Allowing literals to be of multiple types (e.g. |
Original reply by @rudolph9 in cuelang/cue#253 (comment) One consideration (particularly for options 3 and 4), importing cue specs into other languages is to use field attribute to indicate a float type.
|
Original reply by @rudolph9 in cuelang/cue#253 (comment)
@mpvl is there an example analogous to "0.0 would still not be assignable to int, but 0 would be assignable to number and 0.0 and 0 would unify to 0.0"? If I'm reading this correctly (please correct me if I'm mistaken) the following would be true:
If my understanding is indeed correct the following three seem out of place since int is an instance of number
|
Original reply by @mpvl in cuelang/cue#253 (comment) @rudolph9 There are various strong reasons to adopt this proposal in some form:
There are some design issues still to be solved. Like having number and int seems inconsistent. Aside from naming, though, it seems good to adopt this. |
Original reply by @mpvl in cuelang/cue#253 (comment) Another data point: The Go json unmarshaler doesn't allow decoding 1.0 into an int. Atm, I'm leaving to option 3. Open to evidence it should be 4 though, especially if this is required by JSON schema compatibility for instance. |
Original reply by @mpvl in cuelang/cue#253 (comment) There seems to be no consistency in the JSON schema world on this matter: The precise treatment of the “integer” type may depend on the implementation of your JSON Schema validator. JavaScript (and thus also JSON) does not have distinct types for integers and floating-point values. Therefore, JSON Schema can not use type alone to distinguish between integers and non-integers. The JSON Schema specification recommends, but does not require, that validators use the mathematical value to determine whether a number is an integer, and not the type alone. Therefore, there is some disagreement between validators on this point. For example, a JavaScript-based validator may accept 1.0 as an integer, whereas the Python-based jsonschema does not. http://json-schema.org/understanding-json-schema/reference/numeric.html But based on this I see further evidence to chose 3. |
Original reply by @rudolph9 in cuelang/cue#253 (comment)
|
Original reply by @mpvl in cuelang/cue#253 (comment)
A big benefit to keep In general, it is a good idea that if CUE interprets a value in a certain way, it can also be represented that way in CUE itself. Right now, that can be done as When we collapse the two numeric types, there is now way to represent this in CUE itself.
That really depends on where you are coming from and I don't agree this is generally true. The decision on this should be guided by usability, practice, and how things fit in the value lattice. I think there are good arguments to allow |
Original reply by @extemporalgenome in cuelang/cue#253 (comment)
But wouldn't options 3 and 4 change this arrangement, and thus reopen the discussion?
Would it be a problem to defer inferring the type of an integer literal until it is determined that no other constraints prohibit the unification with int (or alternatively, it is determined that there are any constraints that explicitly constrain it to an int)?
This seems much cleaner and more flexible to me, particularly if arbitrary precision constraints can be encoded somehow, perhaps with a new operator (or clever mathematical use of existing syntax), for example: "#Currency: " or "#Even: <this number has -1 fractional digits of precision)" This would also work well with the internal number representation switching from arbitrary-position base-2 floats to arbitrary precision rationals, since JSON does not mandate any particular interpretation of range or precision (and before 2014, none of the JSON RFCs I've seen even provide guidance on interoperability). I've seen applications that do treat some JSON numbers as having arbitrary decimal precision, for example (sometimes encoded in strings, sometimes not), where by contract or agreement, client and server agree on the arbitrary precision interpretation. It would be quite powerful if CUE could drop the binary number assumption in the core type, leaving the role of precision and base to just constraints. I am not a mathematician, so I do not know what the practical implications of this would be: would we limit arithmetic to having no more precision, even temporary, than the constraint allows, or would the result merely need to be no more precise (no rounding) than the constraint dictates? I believe some hard, but solvable, problems with making int a precision constraint are:
If the above could be solved and are worth solving, I believe option 4 would lead to a better language than option 3, though option 3 sounds like it would also result in a better language than we have today. |
Raising a question in the context of #1883. In that issue, I have proposed a mode in which any dropping down to float precision should result in an error (unless explicitly cast to |
The playground was initially developed as a proof of concept over a weekend, in a separate repo/module. However, it has proved to be quite popular. The current situation involves a rather awkward "dance" of running scripts from the playground module within the cuelang.org main module. Having this additional dependency is unncessary, and introduces more development effort. Moving the playground code to the same repository helps eliminate a dependency, and hence the admin associated with maintaining that additional dependency. As part of some enhancements for the GerritHub -> GitHub setup for the main CUE repositroy, we need to establish a new serverless function. By eliminating the playground dependency and housing the code alongside the cuelang.org site, we can simplify our approach to serverless functions, making the addition of future serverless functions easier, including that required to improve the GerritHub -> GitHub setup. This change is incorporates the playground alongside the cuelang.org site. The contents of play/ very closely mirror the contents of github.com/cue-sh/[email protected]. We detail exceptions below. There is still tidy up that can be done, but in the interests of making minimal changes to the playground code, that is deferred for a later date. * .gitignore and .gitattributes files from both repositories have been consolidated. * The GitHub test workflow from both repositories have been combined. * /serve.sh is added to make it easier to locally serve the cuelang.org Hugo site. * /play/run.sh is added to make it easier to locally serve the playground. * /build.sh is changed to reflect the fact the playground is no longer a dependency. * /play/README.md is identical to the playground source version, but for dropping the instructions on how to develop and run the playground locally, instructions which are now in /README.md * /play/dist.sh is slimmed down to reflect the code that was extracted to /play/run.sh. * /play/go.{mod,sum} are exact copies of the playground originals, but appear as diffs because /play/go.{mod,sum} existed before in order to capture the playground dependency separate from the cuelang.org dependencies. This previously allowed the playground to depend on a different version of CUE to the main cuelang.org site. This separation remains now the playground is incorporated with the main cuelang.org site for the same reason. * /play/package.json is changed slightly to drop the "serve" command for the playground in favour of "watch". Because with the playground incorporated into the cuelang.org repo, the hugo command serves the site and playground. Hence the watch command allows for hot-reload-like behaviour of the playground portion. * /play/tools.go is deleted; this was previously used to declare the dependency on the playground module. * /play/webpack.config.js is updated from the playground original to alter the distRoot. * The cuelang.org README is updated to reflect how to develop and run the entire site locally, including the playground. Further tidy up is captured in cue-lang#253. For reference, the diff between the playground repo at 4c9bd3cf5202 and the play directory in this commit is: Only in /home/myitcv/cue/playground/: .git Only in /home/myitcv/cue/playground/: .gitattributes Only in /home/myitcv/cue/playground/: .github Only in /home/myitcv/cue/playground/: .gitignore Only in /home/myitcv/cue/playground/: LICENSE diff -ruw /home/myitcv/cue/playground/README.md ./play/README.md --- /home/myitcv/cue/playground/README.md 2021-11-22 12:33:13.436985000 +0000 +++ ./play/README.md 2022-07-05 19:41:07.728977874 +0100 @@ -20,16 +20,6 @@ The output is the JSON-marshalled result of the CUE input. -### Developing locally - -To develop the application locally, within the `play` directory at the repo root: - -```bash -# Running dist.sh outside of the netlify environment works in a "dev" -# mode which ultimately results in running npm run serve -./dist.sh -``` - ### Details * The TypeScript single-page application entry point is `src/index.tsx` diff -ruw /home/myitcv/cue/playground/dist.sh ./play/dist.sh --- /home/myitcv/cue/playground/dist.sh 2022-07-05 09:28:18.932827261 +0100 +++ ./play/dist.sh 2022-07-12 05:35:19.806465471 +0100 @@ -15,12 +15,6 @@ # be a cache directory # * CUELANG_ORG_DIST - the directory into which we should run dist -if [ "${NETLIFY:-}" != "true" ] -then - trap "trap - SIGTERM && kill -- -$$" SIGINT SIGTERM EXIT - echo "running in 'dev' mode" -fi - # cd to the directory containing the script cd "$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" @@ -48,27 +42,13 @@ rsync -a node_modules/ $NETLIFY_BUILD_BASE/cache/playground_node_modules fi -if [ "${NETLIFY:-}" == "true" ] -then # Dist echo "Install serverless functions" go install -tags netlify github.com/cue-sh/playground/functions/snippets -else - echo "Running serverless functions (background)" - go run github.com/cue-sh/playground/functions/snippets >/dev/null 2>&1 & -fi echo "Building WASM backend" GOOS=js GOARCH=wasm go build -o main.wasm cp $(go env GOROOT)/misc/wasm/wasm_exec.js ./src - -if [ "${NETLIFY:-}" == "true" ] -then echo "Running dist into $CUELANG_ORG_DIST" npm run dist -else - echo "Running npm run serve" - npm run serve -fi - diff -ruw /home/myitcv/cue/playground/package.json ./play/package.json --- /home/myitcv/cue/playground/package.json 2022-07-05 20:38:34.100123583 +0100 +++ ./play/package.json 2022-07-05 14:08:15.559549887 +0100 @@ -31,6 +31,6 @@ "name": "CUEplayground", "scripts": { "dist": "webpack --mode production", - "serve": "webpack serve --mode development --open --hot --host 0.0.0.0" + "watch": "webpack watch --mode development" } } Only in ./play/: run.sh diff -ruw /home/myitcv/cue/playground/webpack.config.js ./play/webpack.config.js --- /home/myitcv/cue/playground/webpack.config.js 2022-07-05 20:38:34.100123583 +0100 +++ ./play/webpack.config.js 2022-07-12 05:35:19.810465740 +0100 @@ -2,7 +2,7 @@ const HtmlWebpackPlugin = require('html-webpack-plugin'); -let distRoot = __dirname; +let distRoot = path.join(__dirname, ".."); if (process.env.NETLIFY == "true") { if (process.env.CUELANG_ORG_DIST == "") { Signed-off-by: Paul Jolly <[email protected]>
Just found this because it was extremely confusing that an integer was not validating with the following cue: given:
this was not valid:
which seems wrong to me. I appreciate the complexity of this issue and typing, but its a bit frustrating to have to specify |
Curious if any work is being done regarding this? We are currently having to educate everyone to use Or is there a way to parse all ctx := cuecontext.New()
instances := load.Instances([]string{"foo.cue"}, nil)
v := ctx.BuildInstance(instances[0])
// Everything below is from imagination, looking for something like this:
v.Walk(func(x cue.Value) {
if x.Kind() == cue.FloatKind {
x.Kind = cue.NumberKind
}
}) I've tried digging through the source code for a bit, and it seems like
compiles to an |
JSON does not make a distinction between floats and ints, and there are tests in the jsonschema test suite that check that it's OK to use floating point literals in primitives that expect an integer. So accept whole floats anywhere we need a uint. Relevant issue: #253. Signed-off-by: Roger Peppe <[email protected]> Change-Id: I44632a6eadd2aeaff626b27ca6536d4aa47035aa Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200939 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
Originally opened by @rudolph9 in cuelang/cue#253
Background
JSON has no concepts of
float
orint
onlynumber
. The current behavior of cuelang hasfloat
andint
as completely disjoint branches on the value lattice. Further, any concrete number with a dot.
implicitly is afloat
(e.g.0.0
) and any number without a dot implicitly is anint
(e.g.0
).This causes an error in the following example:
A couple tickets related to this behavior:
cuelang/cue#252
cuelang/cue#233
Objective
Unify
float
andint
as simplynumber
which would make for example{foo: 0.0} & {foo: 0}
valid cuelang.Transition
Phase 1:
float
to be synonymous withnumber
.int
to declare a number with a constraint that does not allow decimal values.0
,0.0
,1.2,
5`, etc.) implicitly declare a number.Phase 2:
float
deprecatedcue fmt
to change all usages offloat
tonumber
Phase 3:
float
The text was updated successfully, but these errors were encountered: