Skip to content

Conversation

m-maryia
Copy link
Collaborator

@m-maryia m-maryia commented Oct 7, 2025

  • Add JP parsing rules and metadata without adding a separate JP address model.
  • Handle the absence of model in model.py, renderer.py, parsing_module.py, formatting_module.py in order to make it possible to generate the parsing rules when country model is not defined.
  • Define the legacy country code constant since the legacy rules need to be used for countries without a defined model.

Copy link
Collaborator

@norgevz norgevz left a comment

Choose a reason for hiding this comment

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

Could you add a description on the PR outlining all the changes produced here?

@m-maryia
Copy link
Collaborator Author

m-maryia commented Oct 8, 2025

Could you add a description on the PR outlining all the changes produced here?

Done

Copy link
Collaborator

@norgevz norgevz left a comment

Choose a reason for hiding this comment

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

Almost there!

model/main.py Outdated
all_token_content = renderer.wrap_all_token_details(all_token_content)
content += all_token_content
model = renderer.get_model(country)
if model is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just have if model:here? It is simpler to parse that is not None.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can, I replaced it by if model: and did the same in other files.


countries = []
vendor_extension_extra_pages: List[ExtraPage] = []
LEGACY_COUNTRY_CODE = "XX"
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 defined, but not used. Why do we need this?

Copy link
Collaborator Author

@m-maryia m-maryia Oct 10, 2025

Choose a reason for hiding this comment

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

This is used in other CL (related to this one) that I also sent for review.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's define this where it's used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

@norgevz norgevz left a comment

Choose a reason for hiding this comment

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

LGTM % removing the unused constant.


countries = []
vendor_extension_extra_pages: List[ExtraPage] = []
LEGACY_COUNTRY_CODE = "XX"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's define this where it's used.

@m-maryia m-maryia merged commit e065fb5 into battre:main Oct 13, 2025
2 checks passed
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