open-contracting / cove-ocds Goto Github PK
View Code? Open in Web Editor NEWOCDS Data Review Tool
Home Page: https://ocds-data-review-tool.readthedocs.io
License: Other
OCDS Data Review Tool
Home Page: https://ocds-data-review-tool.readthedocs.io
License: Other
I've been making changes to Key Field Information, and this requires the same tests to be carried out in lib-cove-ocds and cove-ocds.
The tests are necessary in both places:
It would save a lot of lines of code if we were able to share the tests in some way, while still carrying them out in both places.
Hello!
As we are about to publish French award data, I had to validate it (4 days ago): https://standard.open-contracting.org/review/data/55390859-63fd-453a-989b-21c612d69687
If you clicked the above link and the validation have not expired:
It's not surprising, it's trying to display 120,000+ releases.
I don't think that displaying a release table with so many lines is relevant, especially if it costs so much on both the client and the server side.
Would it make sense to disable the display of the release table from a certain number of releases?
More generally, should the reviewing process be optimized for big files? That could be changes on cove-ocds, but also the release of command line tool that would be run locally.
Feature request from OpenDataServices/cove#263
Original text:
When a file containing multiple releases is validated, the total count of items and documents are shown. For clarity, could we also include the unique count of:
- items based on id
By showing that we found 19 mentions of a document and 4 unique document ids, publishers can be check the release has been validated correctly.
Some quick notes from a discussion with @kindly on this today:
The primary use case we have in mind is publishers sharing draft data with the helpdesk in record package format and helpdesk analysts wanting to flatten this to help give feedback (e.g. it's easier to check all the values of a given field by looking at a spreadsheet than by reviewing a JSON file).
Records have the following components:
oneOf
and it isn't possible to definitely tell which has been provided. Also the list could be mixed. Would require more work.
We also discussed whether compiled and embedded releases should be flattened into the same spreadsheet.
We also discussed whether this would sit better in CoVE/flatten-tool or OCDSKit/-web (possibly via the tabulate command supporting a spreadsheet output).
Seeking feedback from @yolile @romifz @jpmckinney @mrshll1001@pindec and others on:
Clicking the collapse button on the second 'structural errors' element collapses the first 'structural errors element', so it isn't possible to collapse the second one (example).
From open-contracting/cove-oc4ids#20
If the value is a string, it should be surrounded by quotation marks. This makes it easier to see that e.g. 500 is "500"
instead of 500
. It also means something will be printed to the screen for the empty string (""
instead of nothing).
Moved from OpenDataServices/cove#1198
See OpenDataServices/cove#1117 for how we did this for 360Giving.
Right now, all of the validation errors are presented in one table. Grouping these by type (eg missing-but-required, format errors, 'other') helps people understand what kind of things they need to do in order to make their data use the standard.
The task here is to:
The data has duplicate values in tender/participationFees/0/methodOfPayment
which is an array of strings, but no examples are shown in the first 3 examples column:
For arrays of strings, I think the examples should just show the whole field so that users can see that there are duplicates, e.g.
First 3 errors
DD - Demand Draft;FDR - Fixed Deposit;DD - Demand Draft;FDR - Fixed Deposit
DD - Demand Draft;FDR - Fixed Deposit;DD - Demand Draft;FDR - Fixed Deposit
DD - Demand Draft;BC - Bankers Cheque;SS - Small Savings Instrument;FDR - Fixed Deposit;DD - Demand Draft;BC - Bankers Cheque;SS - Small Savings Instrument;FDR - Fixed Deposit
https://standard.open-contracting.org/review/terms/#privacy_notice says "The experiment runs from 4th March to 30th September 2020."
Copied from OpenDataServices/cove#263 (comment)
KFI is an important part of how the Helpdesk supports publishers to understand their data but to an unsupported user who is still learning about the standard the terse presentation of the stats isn't helpful.
Inline docs could be useful here.
Original text:
I have a great deal of difficulty understanding how each of the numbers is calculated (using the linked file as input). For example, 8 releases have an (identical) planning field, and the number is 1.
However, I think we first need to determine how frequently this section is referenced, before investing time in improving its clarity.
lib-cove is very slow: open-contracting/lib-cove-oc4ids#23
There is no reason to re-validate the data on each request. This should be cached.
I think we can just use the per-site cache, which is i18n-aware: https://docs.djangoproject.com/en/3.1/topics/cache/#the-per-site-cache Please setup memcached on the server.
Can we check the web hooks?
From discussion with @ColinMaudry in Georgia:
We don't signpost the command line tool anywhere from the validator landing page.
At the moment this is a beta tool but it would be good to let users know it exists.
Could we add a link?
In the future we might want to impose a limit on upload size for the web tool and direct users to the command line tool for large files (rather than returning a server error).
Follow-up to #31, which yielded #35.
Large, invalid files yield very large response sizes (performance), and more information than is useful (UX), e.g. a list of 42,000 invalid entries for one error type.
File | Valid? | File Size | Response size before PR | Response size after releases table PR #35 |
---|---|---|---|---|
repeated_errors_repeated.json (not public data) | Invalid | 359MB | 65.8 MB | 27.32 MB |
badfile_repeated.json (script to generate) | Invalid | 341MB | 174.18 MB | 150 MB |
Test files are now here: https://github.com/open-contracting/sample-data-private/tree/master/data
I think we can have a configurable setting to limit the number of results returned.
To address performance issues, we can set a high limit that still exceeds usefulness, like 1000.
To address usability issues, we can have a smaller number like 100. We'll want to randomize the results returned, so that we're not simply reporting e.g. the first 100 errors all caused by old data and none of the errors caused by newer data (publishers who are only making improvements to new data are likely to ignore the results if they only seem to pertain to old data).
CoVE is used by a number of different types of users, each attempting to achieve different things.
Through our research last year, and Georg's research into personas for the OCP web presence, we can have a reasonable go at documenting a handful of these, walking through their paths to interact with the software, and improve things for each of them as we go.
Internal - see https://docs.google.com/document/d/1KQ-j4q0rC5lkhIHGk9bCC_TA50PGP5KSvTZaKUuuaaQ/edit#bookmark=id.55uduidgj3bu
Packages (schema: "A list of URIs of all the release packages that were used to create this record package") look like this:
"packages": [
"https://budeshi.ng/api/releases/1288/planning",
"https://budeshi.ng/api/releases/1288/tender",
"https://budeshi.ng/api/releases/1288/planning",
"https://budeshi.ng/api/releases/1288/contract"
]
DRT report shows a structural error: Array has non-unique elements
Moved from OpenDataServices/cove#1245
From open-contracting/cove-oc4ids#19
The form prompts can be improved so that a first-time user knows what to do without scrolling below the fold to read the instructions, for example: "Paste an OCDS release package or record package as JSON" instead of simply "Paste". I can suggest more improvements, but I recommend that either @mrshll1001 or @pindec do a quick review to pick up these types of usability issues for first-time users.
Similarly, the order and flow of content below the form doesn't put the most important or relevant information first.
For example, I would at minimum swap the positions of the "Check and Review" and "About OCDS" blocks. Ideally, "About OCDS" would come after both other blocks, as it's the least relevant content; anyone arriving at the OCDS Data Review Tool is very likely to at least know what OCDS is.
The text in each box can also be clearer, simpler, more straight-forward for first-time users. As in issue description, I think others should do a first pass.
Noting some quick observations from Hotjar recordings. In general, drawing conclusions from recordings over a short period will be biased, as the recordings will tend to be the same user working to implement OCDS.
Moved from OpenDataServices/cove#1197
In the explore_additional_content
block, explore_record
has "Is structurally correct?", "Number of records", etc. which are in the headlines block in explore_release
. The record template also has the Schema and Convert boxes alongside the Headlines box, whereas the release template has them beneath.
As much as possible, the two templates should be the same. We should perhaps have each inherit from (or reuse blocks from) a common template.
Once this and open-contracting/cove-oc4ids#43 are closed, we can:
#35 (comment) has links to test data and scripts to generate data.
If the data can't be made public, we should create a private repository to put the data and scripts.
cc @Bjwebb
https://sentry.io/open-data-services/cove/issues/392837329/
TypeError: datetime.datetime(2017, 1, 1, 0, 0) is not JSON serializable
(5 additional frame(s) were not displayed)
...
File "cove/lib/exceptions.py", line 53, in wrapper
return func(request, *args, **kwargs)
File "cove/lib/converters.py", line 92, in convert_spreadsheet
**flattentool_options
datetime.datetime(2017, 1, 1, 0, 0) is not JSON serializable
Example file: date_in_header.xlsx
Run coverage run --source cove_ocds,cove_project manage.py test
/opt/hostedtoolcache/Python/3.6.12/x64/lib/python3.6/site-packages/cove/settings.py:31: UserWarning: SECRET_KEY should be added to Environment Variables. Random key will be used instead.
warnings.warn('SECRET_KEY should be added to Environment Variables. Random key will be used instead.')
System check identified no issues (0 silenced).
----------------------------------------------------------------------
Ran 0 tests in 0.000s
OK
When migrating from Travis to Github CI's, I left it in as I wasn't certain it does nothing, and if felt like we should check that. If it really does nothing, we can remove. I suspect coveralls data comes from the next test line anyway.
From a helpdesk issue
Currently when reporting structural errors the review tool reports the location of the error relative to the package e.g. releases/38/tender/tenderers
. Since some packages are generated on the fly, and there may be complex mapping behind them, it may be helpful if it would also extract the OCID of the release with the issue rather than just its position in the array. This would support the publisher finding the offending release elsewhere to make the change.
Copied across from OpenDataServices/cove#320
The original issue has some discussion, but pertinent bits:
It's certainly true that we don't intend CoVE / lib-cove-ocds to be used as part of the backend for a web site - it's definitely not fast enough. If there was a demand for this, we could probably concoct something - but I'd want to know more about exactly what was required. As it stands, I'm only aware of @patxiworks wanting to use CoVE in this way, and unfortunately we weren't able to support what he wanted to do.
However, a web API to CoVE also has a role in publication systems, and potentially in data consumption applications, as part of automated processing, where response time isn't as important. Packaging up something that can be run as a 'black box' and handle requests for feedback on data that goes beyond simple validation can be useful.
and
I guess the use case for a web API is an implementer who is unable or unwilling to use lib-cove-ocds as a library. (Even if their system is implemented in another language, they can either bridge to Python, or shell out to the libcoveocds command.) Right now, the demand for that seems low, but the issue can certainly remain open.
This should be configureable as an environment variable.
From #35 (comment)
It looks like this check currently only looks at the release, not the package metadata, at least a blank string publisher/scheme
is not reported by the check.
The check should be carried out on both the package metadata and the contents of the package.
I'm not sure how this is implemented, so worth checking if this issue is specific to this check or whether it applies to the way additional checks work in general.
Introduced as part of OpenDataServices/cove#895 when we replaced the monolithic oneOf
validation messages with the individual messages for each subschema.
Errors about date
and tag
being required can come from either subschema, and are repeated for each.
This is only a problem for files that have a mix of assumed embedded and linked releases for different records.
In https://ocds-data-review-tool.readthedocs.io/en/latest/translations.html
See https://docs.transifex.com/client/pull
-f or --force: Force the download of the translations files regardless of whether timestamps on the local computer are newer than those on the server.
I just had an issue where because of git branch switching (I assume), it was skipping ES and I wasn't getting the latest translations. I had to add -f to get it to do that.
Is there any situations where this is bad? If not, should we just add that to the docs?
Add a note to the Headlines like: "This file contains compiled releases [and versioned releases]. If you created these merged releases, we encourage you to test your implementation of the merge routine using the OCDS Merge test cases. For assistance, please contact the OCDS Helpdesk."
Moved from OpenDataServices/cove#1210
e.g. https://standard.open-contracting.org/review/data/704b068b-af6b-4dbd-9001-e0430e2a8e16
It is just confusing and distracts from the main message.
@robredpath commented:
Agreed. We could probably make some use of that space, though - with some custom text that's helpful to someone who's come to an expired results page!
Moved from OpenDataServices/cove#1209
When viewing validation results, if you try to collapse the Additional Fields
panel, it's header and description overrides the titles and descriptions on all of the other panels. This doesn't happen when collapsing/restoring the other panels, only the Additional Fields
panel.
As well, when there are no additional fields in use, it might be good to specify a row in that panel to say that no fields were identified rather than it just being blank as it looks like something might be missing.
From OpenDataServices/cove#951
We did this for all other validation messages, but required fields are more difficult because current the context path is the parent.
Follow-up to OpenDataServices/cove#896
If there is a use case, we can have a "See all" link/button to download them all. (This button could then similarly be implemented for the Records Table and Releases Table.)
In OpenDataServices/cove#895 we now assume whether a record has linked or embedded releases, in order to use the correct subschema within the oneOf
block. Text about this assumption is added to every relevant validation message.
This text is repeated for each validation error message. Instead we should group the messages by subschema used, and state the assumption only once.
From CRM-5858, the data URL is https://www.data.gouv.fr/fr/datasets/r/68bd2001-3420-4d94-bc49-c90878df322c
Submitting that URL generates a 5MB HTML file with tens of thousands of rows of organization names.
Similar to #57, we can implement a "See all" download if there is a use case.
When there are errors in the validation, the error count column shows the errors for the row.
However, the number does not look like a link.
Perhaps a call to action is needed or format the number to look like a link
Example: http://standard.open-contracting.org/validator/data/f7d35e4c-d3c8-4a3b-887c-539ceff7a491
Copied from OpenDataServices/cove#118
Although we provide a list of fields that are present but aren't in the schema (ie, "additional fields"), we don't provide a list of fields that are in the schema but aren't in the file, or any other way for people to discover fields that may be relevant to them from the results page. The risk, of course, is that excessive noise is generated, but some way of allowing people to see their mapping template alongside their existing coverage could help alignment.
In this first iteration, this documentation should cover information that is unique to the DRT (i.e. we should assume that the developer knows Python, Django, JSON Schema, OCDS, etc. which are not unique to the DRT). It should cover the information that developers newly working on the DRT would be given, with respect to the responsibilities of each component (lib-cove-ocds, lib-cove, cove-ocds, cove), for example.
Related reading:
Possible contents:
When validating a record package with embedded releases the Records Table
shows a release count
of 0.
In the case of the sample data I was validating, it should have shown a release count
of 3 - unless I'm entirely misinterpreting the column header.
Link to sample data validation here: https://standard.open-contracting.org/review/data/3a395812-d486-48b7-8789-d0d2e16e3651
http://httpstat.us/404 sometimes returns a different status code, such as 503 or 524, which is probably due to the request taking too long / not returning from the origin server, and Cloudflare making an error response.
A GitHub search shows that we use this in this repo, and kingfisher-collect, so I've also opened an issue there.
The row number reported in spreadsheet validation error messages is incorrect when the #skiprows or #headerrows configuration properties are set.
For example, in the data which generated the following error message (a spreadsheet with #skiprows set to 2 and #headerrows set to 5) the error is actually on row 8, not row 2:
The row reported in the error message should account for the configuration properties set in the source spreadsheet.
See open-contracting/cove-oc4ids#30, replacing "OC4IDS project package" with the appropriate OCDS text (e.g. "OCDS release package").
Moved from OpenDataServices/cove#1196
From OpenDataServices/cove#951. There was some work on this, but it was removed because changes to the docs site weren't merged yet. They now have been, so we can do this. Steps:
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.