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

Add type hints to fields module #1214

Merged
merged 25 commits into from
Oct 4, 2023
Merged

Conversation

liamtoozer
Copy link
Contributor

@liamtoozer liamtoozer commented Sep 26, 2023

What is the context of this PR?

  • Added type hinting to the fields module to align with other modules with type hinting in place.
  • Updates to field handlers to include optional return types for UnboundField
  • Small tweaks to some unit tests to include enforced keyword args and more concrete types

How to review

Check to see if the type hints added to the module make sense, and any advice on how to improve would be great

Checklist

  • New static content marked up for translation
  • Newly defined schema content included in eq-translations repo

Trello

@liamtoozer liamtoozer added the help wanted Extra attention is needed label Sep 26, 2023
Copy link
Contributor

@katie-gardner katie-gardner left a comment

Choose a reason for hiding this comment

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

Looking good 👍 added a few initial comments with a couple of questions/ideas

app/forms/fields/date_field.py Outdated Show resolved Hide resolved
app/forms/fields/integer_field_with_separator.py Outdated Show resolved Hide resolved
app/forms/fields/month_year_date_field.py Outdated Show resolved Hide resolved
app/forms/fields/year_date_field.py Outdated Show resolved Hide resolved
app/forms/fields/year_date_field.py Outdated Show resolved Hide resolved
app/forms/fields/year_date_field.py Outdated Show resolved Hide resolved
app/forms/fields/decimal_field_with_separator.py Outdated Show resolved Hide resolved
app/forms/field_handlers/select_handlers.py Outdated Show resolved Hide resolved
mypy.ini Outdated Show resolved Hide resolved
app/forms/fields/year_date_field.py Outdated Show resolved Hide resolved
app/forms/fields/date_field.py Outdated Show resolved Hide resolved
app/forms/fields/month_year_date_field.py Outdated Show resolved Hide resolved
app/forms/fields/date_field.py Outdated Show resolved Hide resolved
app/forms/fields/date_field.py Outdated Show resolved Hide resolved
app/forms/validators.py Outdated Show resolved Hide resolved
Copy link
Contributor

@MebinAbraham MebinAbraham left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@petechd petechd left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@katie-gardner katie-gardner left a comment

Choose a reason for hiding this comment

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

🎉

@liamtoozer liamtoozer merged commit 09b1c22 into main Oct 4, 2023
15 checks passed
@liamtoozer liamtoozer deleted the add-type-hints-to-fields-module branch October 4, 2023 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants