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

Integrate Python address parser using use-address library #56

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Jbony1988
Copy link

@Jbony1988 Jbony1988 commented Aug 5, 2024

Add usaddress library to project dependencies

  • Implement AddressParse API view for address parsing
  • Create parse() method to handle address parsing logic
  • Update frontend to send requests to new API endpoint
  • Display parsed address components and handle errors in UI
  • Add error handling for unparseable addresses
  • Update tests to cover new address parsing functionality"

This PR involves recreating an address parsing form similar to DataMade's Parserator web service. Parserator is a tool that takes input strings representing addresses (e.g., "123 main st chicago il") and splits them into their component parts, such as street number, street name, city, and state. Here's a breakdown of what this entails:

How to test this PR

  • After checking out this branch, follow these steps:

  • Setup:

  • Ensure Docker and Docker Compose are installed on your machine

  • Build the application containers:

run command in project docker-compose build

  • Start the application:

    run command in project docker-compose up

  • User Interface Testing:

  • Open http://localhost:8000 in your web browser

  • Test with valid input:

  • Enter "123 main st chicago il" in the input field

  • Click the submit button

  • Expected output:

  • AddressNumber: 123

  • StreetName: main

  • StreetNamePostType: st

  • PlaceName: chicago

  • StateName: il

  • Test with invalid input:

  • Enter "123 main st chicago il 123 main st" in the input field

  • Click the submit button

  • Expected output: An error message should be displayed indicating invalid input

  • Unit Testing:

  • Run the unit tests using:

    docker-compose -f docker-compose.yml -f tests/docker-compose.yml run --rm app

  • Expected output: All tests should pass, and no linting errors should be reported

  • Additional Notes:

  • If you make any changes, you may need to rebuild the containers

  • Ensure the console output doesn't show any unexpected errors during testing

 Add usaddress library to project dependencies
- Implement AddressParse API view for address parsing
- Create parse() method to handle address parsing logic
- Update frontend to send requests to new API endpoint
- Display parsed address components and handle errors in UI
- Add error handling for unparseable addresses
- Update tests to cover new address parsing functionality"
Copy link
Member

@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.

@Jbony1988 nice work here! A few things to address, but overall this is good and well documented.

For your pull request description, would you mind formatting and cleaning up the text a bit? There's still some boilerplate text in there that can be removed.

{% block extra_js %}
<script src="{% static 'js/index.js' %}"></script>
{% endblock %}
<script>
Copy link
Member

Choose a reason for hiding this comment

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

the javascript code should be placed in js/index.js and not added inline to the template. can you move this code over?

Copy link
Author

Choose a reason for hiding this comment

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

this has been resolved


if (data.detail && data.detail.startsWith('Error parsing address:')) {
// Display parsing error
const errorMessage = data.detail.split('ORIGINAL STRING:')[0].trim();
Copy link
Member

Choose a reason for hiding this comment

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

nice work here displaying a useful error response

Copy link
Author

Choose a reason for hiding this comment

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

awesome thanks

@Jbony1988
Copy link
Author

I have cleaned up the description of the PR and I moved the javascript to the js/index.js file

Copy link
Member

@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.

thanks @Jbony1988

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