Skip to content

Comments

DataMade code challenge submission#1

Open
haowens wants to merge 4 commits intomasterfrom
submission
Open

DataMade code challenge submission#1
haowens wants to merge 4 commits intomasterfrom
submission

Conversation

@haowens
Copy link
Owner

@haowens haowens commented Jul 23, 2024

Overview

This PR is the filled in implementation of basic functionality of DataMade's Parserator [which takes input strings that represent addresses (like 123 main st chicago il) and displays a table of the address broken up into its component parts]. This implementation also displays an error message if the imput string is invalid. Additionally, this PR contains a couple basic tests that verify the API and views.py methods are returning the correct json.

Demo

Screen.Recording.2024-08-05.at.5.22.27.p.m.mov

Notes

Testing Instructions

To run the code in local development:

  • build the dev container: docker-compose build
  • start the dev server: docker-compose up
  • I believe this project follows Django directory conventional structure, in terms of where relevant code is located in this repository

To test the API

  • tests can be added or tweaked in tests/test_views.py
  • to run the tests in that file: docker-compose -f docker-compose.yml -f tests/docker-compose.yml run --rm app

Copy link

@derekeder derekeder left a comment

Choose a reason for hiding this comment

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

@haowens thanks for submitting this PR! Overall, the code is clean and works as expected. I have a few small comments to address.

Also, could you provide a more detailed description in your pull request? We'd like to see a summary of the changes you made, a screenshot or demo description of it working, and instructions on how to run and test the code.

@@ -0,0 +1,12 @@
# To get started with Dependabot version updates, you'll need to specify which

Choose a reason for hiding this comment

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

can you share why this was added?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Bizarrely I have no clue. Initially assumed it was from trying different things to fix the npm install error that was coming up when I was first trying to build the dev container, but seems like I merged that to master without this file being there. Definitely didn't manually add it because I haven't consciously worked with/really thought about github's "dependabot" before. Will delete and pay more attention to staged changes in the future !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants