-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add more realistic test data #1141
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one minor code suggestion. Aside from that, I have some comments on the test data:
- Since the test data is from OSM and I think it is a substantial extract, we should honor the OSM license by indicating somewhere that the data is from OSM and that it is licensed under ODbL.
- Why not convert londondata.txt into a full-fledged GeoJSON file so that it can be displayed on a map using GitHub's GeoJSON display feature. The GeoJSON file can also include the attribution/license mentioned above as an attribute.
cadasta/core/fixtures/__init__.py
Outdated
break | ||
|
||
name = 'Spatial Unit #{}'.format(i) | ||
type = random.choice([c[0] for c in TYPE_CHOICES]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not create the choices list as a variable outside the loop so that we don't recreate the list on every loop iteration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On it...
I deliberately decided against using GeoJSON. We only need the geometries, so I wanted to have a simple format that only provides the geometries so I can read the file on line at a time. GeoJSON has some overhead that I would need to remove; it adds complexity to the code that is not necessary. I also don't see what we get from being able to view the geometries on a map on Github. I agree on adding the licensing information, I will add it to the top of the file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Proposed changes in this pull request
This PR fixes a minor annoyance. The test project London 2 has its project extent in London but its test data is in Germany. The test data itself is just a grid of rectangles.
The changes include:
loadfixtures
command with an option--records
that can be used to specify the maximum number of records loaded into the project. It's usage is./manage.py loadfixtures --records=10000
.core/fixtures
When should this PR be merged
Anytime
Risks
None
Follow up actions
None
Checklist (for reviewing)
General
migration
label if a new migration is added.Functionality
Code
Tests
Documentation