Skip to content
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

Lots: Error when maximumLotsBidPerSupplier is set to infinity (1e9999) #128

Open
jpmckinney opened this issue Jan 27, 2020 · 4 comments
Open
Assignees
Labels
bug Core Relates to a recommended extension Schema Involves editing the schema
Milestone

Comments

@jpmckinney
Copy link
Member

jpmckinney commented Jan 27, 2020

The OCDS for EU profile maps maximumLotsBidPerSupplier to 1e9999 if there is no limit to the number of lots for which a bid can be submitted.

This mapping was preferred to a new boolean field, because it reused an existing field, while preserving the logical behavior of the field (e.g. comparing this value to the number of lots to which a given bid relates). It was also preferred to setting the field to a large 32-bit integer; while, in reality, no request for tender would have that many lots, and no bid would be related to that many lots, 2,147,483,647 looks like an error, whereas 1e9999 looks deliberate (it's also a common and consistent way to overflow floating-point parsing into infinity).

However, the jsonschema library will error. It considers infinity to be a number, but not an integer:

import json
from jsonschema.validators import Draft4Validator

Draft4Validator({"type":"number"}).validate(json.loads('1e9999'))
# []
Draft4Validator({"type":"integer"}).validate(json.loads('1e9999'))
# ValidationError: '1e9999' is not of type 'integer'

The same is true in Python:

import math
import decimal

isinstance(float('inf'), int)
# False
isinstance(math.inf, int)
# False
isinstance(decimal.Decimal('Infinity'), int)
# False

We can:

  1. Weaken the type of the field to allow numbers, though numbers only make sense for infinity (e.g. you can't bid on 0.3 of a lot).
  2. Use a more complex oneOf rule, in which one branch is specific for infinity. In that case, we'll need to check that the Data Review Tool (and possibly others) can handle it.
  3. Patch the jsonschema library (in the Data Review Tool and elsewhere) to treat infinity as a valid value for an integer field.

I prefer the third option, if this problem is specific to Python. If Java, etc. validators have the same issue, then we should opt for another option.

@jpmckinney jpmckinney added bug Core Relates to a recommended extension Schema Involves editing the schema labels Jan 27, 2020
@jpmckinney
Copy link
Member Author

Update: Same issue in plain Ruby at least:

require 'bigdecimal'

Integer === Float::INFINITY
# false
Float === Float::INFINITY
# true

Integer === BigDecimal('Infinity')
# false
Float === BigDecimal('Infinity')
# false
BigDecimal === BigDecimal('Infinity')
# true

@duncandewhurst
Copy link

I'll look into option 2.

@jpmckinney
Copy link
Member Author

Sounds good

@duncandewhurst duncandewhurst moved this from To do: Extensions to In progress in OCDS 1.2 Oct 24, 2024
@duncandewhurst
Copy link

I added a oneOf with an enum for infinity:

"maximumLotsBidPerSupplier": {
  "title": "Maximum lots per supplier",
  "description": "The maximum number of lots that one supplier can bid on as part of this contracting process.",
  "oneOf": [
    {
      "type": [
        "integer",
        "null"
      ]
    },
    {
      "type": [
        "number"
      ],
      "enum": [
        1e9999
      ]
    }
  ]
},

Invalid data results in an error message that isn't particularly helpful. Users would need to look at the schema to understand how to resolve the error:

image

I think we'll need to update the custom oneOf validator in lib-cove-ocds to handle this specific case or to report the underlying errors for any use of oneOf. Alternatively, in the CoVE instance for the Risk Data Library, we implemented a field-specific custom error message for that schema's use of oneOf: https://github.com/GFDRR/rdls-cove/blob/9b69d274eea60afb612e2612a1a7cffb2815d3ee/cove_rdls/templates/cove_rdls/validation_table.html#L89-L90.

Data Review Tool results for different values:

I also tried using anyOf instead of oneOf, which resulted in the same error message, as expected. The enum could be replaced with maximum and minimum, but that won't make any difference either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Core Relates to a recommended extension Schema Involves editing the schema
Projects
Status: In progress
Development

No branches or pull requests

2 participants