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

SCRIPTS: Add script to merge geojson files and format them in a MapRoulette tag fix friendly way #376

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

tsmock
Copy link
Collaborator

@tsmock tsmock commented Sep 30, 2020

Description:

This is a script I wrote awhile back to convert multiple geojson files into a single geojson file. To help with uploading to MapRoulette, there is also a command line flag that converts the geojson files into RFC7464 compliant line-by-line geojson. As a followup on #361 , I modified the script to also convert basic fix_suggestions into MapRoulette cooperative challenges when outputting RFC7464 compliant geojson in order to help me validate the fix suggestion changes I made to the InvalidTagsCheck class.

There are a couple of TODO comments scattered throughout the file, mostly to do with investigating some edge cases (will Polygons always be a way, or may they occasionally be a relation? -- looking at identifiers would probably work for this as well).

This is partially related to #365 .

Potential Impact:

N/A

Unit Test Approach:

N/A

Test Results:

N/A

…ulette tag fix friendly way

Signed-off-by: Taylor Smock <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Sep 30, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Collaborator

@danielduhh danielduhh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice!
Is there any reason why you're using the geojson output as opposed to the .log files that are already line-delimited? I know the log files unfortunately aren't separated by check and take some work to get the files organized that way.

return [g for g in files if re.search(r".*\.geojson(|\.gz)$", g)]


def fix_suggestions_to_cooperativeWork(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks for adding it.
We're working on bringing in a pyatlas-checks library to house some MapRoulette related code integrated with the maproulette-python-client.

Once we have that checked in, we can repurpose this code within that cli.

Also.. check out this ticket: osmlab/maproulette-python-client#63. If you have time, it would be awesome if you could additionally add a Cooperative Challenge model for the python client.

@danielduhh danielduhh added the script Python script label Oct 2, 2020
@tsmock
Copy link
Collaborator Author

tsmock commented Oct 2, 2020

Is there any reason why you're using the geojson output as opposed to the .log files that are already line-delimited? I know the log files unfortunately aren't separated by check and take some work to get the files organized that way.

Mostly because it was easier to work with organized checks to create MapRoulette tasks (when I first made the script, all I wanted to do was merge a specific check's files together -- I often run multiple checks instead of just the one I'm working on, to make certain I didn't accidentally break something). It eventually grew into what it is now, step by step. I added the write_feature_rfc7464 function when I had some issues uploading rather large files to MapRoulette, and then I added the fix_suggestions_to_cooperativeWork function when I wanted to ensure that the fixes I was working on would "just work (tm)".

I didn't know we had a python client repository (but I didn't look either -- I wrongly assumed that everything related to atlas-checks would be in this repo).

fh.write(ASCII_RECORD_SEPARATOR)

with open(file_name, "a") as fh:
string = geojson.dumps(fc)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing this is just calling json.dumps, in which case we probably want to set ensure_ascii=False to maintain unicode characters.

parser.add_argument("--line-by-line", action="store_true")
args = parser.parse_args()
for f in args.files:
logging.info(f)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: more context in this log would be good

description="Create a line-by-line geojson file"
)
parser.add_argument("files", nargs="+")
parser.add_argument("-c", "--checks", nargs="?")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the use of this argument it appears to be expecting a single check name. So I think it would make more sense to have the flag be --check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
script Python script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants