Skip to content

Add ability to specify output format for DurationField#8532

Merged
peterthomassen merged 6 commits into
encode:masterfrom
sevdog:duration-format
Aug 12, 2025
Merged

Add ability to specify output format for DurationField#8532
peterthomassen merged 6 commits into
encode:masterfrom
sevdog:duration-format

Conversation

@sevdog

@sevdog sevdog commented Jun 24, 2022

Copy link
Copy Markdown
Contributor

Description

As explained in #8527 this PR will provide the ability to choose the output format for DurationFields.

Since Python builtim timedelta object does not have any format function we are going to allow only formats which are already defined in Django codebase in the package django.utils.duration:

  • duration_string
  • duration_iso_string

Comment thread docs/api-guide/fields.md Outdated
Comment thread docs/api-guide/settings.md Outdated
@lovelydinosaur

Copy link
Copy Markdown
Contributor

Thanks - good to talk this through from a docs-first perspective.

@stale

stale Bot commented Oct 16, 2022

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Oct 16, 2022
@stale stale Bot closed this Nov 1, 2022
@pietzschke

Copy link
Copy Markdown

Can we reopen this?

@auvipy auvipy reopened this Jun 4, 2024
@stale stale Bot removed the stale label Jun 4, 2024
@pietzschke

Copy link
Copy Markdown

I think it's not possible to contribute to the pr to solve the conflicts, @sevdog would you be able to do it?
Otherwise i will create a new one and do the fixes.

@sevdog sevdog force-pushed the duration-format branch from 9d506ba to 9713c1d Compare June 5, 2024 09:28
@sevdog

sevdog commented Jun 5, 2024

Copy link
Copy Markdown
Contributor Author

Just rebased the PR, the "conflict" was a very silly issue in the doc file 🤷‍♂️

CI is failing due to a change in django-main branch, it should be addressed in an other PR.

@sevdog sevdog marked this pull request as ready for review June 5, 2024 09:33
@sevdog sevdog force-pushed the duration-format branch 2 times, most recently from 295a462 to 70751a5 Compare July 8, 2024 12:41
auvipy
auvipy previously requested changes Jul 9, 2024

@auvipy auvipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I saw FAILED tests/test_fields.py::TestISOOutputFormatDurationField::test_outputs
=========== 1 failed,

@sevdog sevdog force-pushed the duration-format branch from 70751a5 to 35398b3 Compare July 9, 2024 16:36
@sevdog

sevdog commented Jul 9, 2024

Copy link
Copy Markdown
Contributor Author

Ok, fixed the test. It was a long lasting double typo (wrong class and wrong format).

@auvipy auvipy requested a review from a team July 10, 2024 09:24
peterthomassen
peterthomassen previously approved these changes Jul 10, 2024
Comment thread docs/api-guide/fields.md Outdated
@sevdog sevdog force-pushed the duration-format branch from 35398b3 to bcc4ebf Compare July 11, 2024 06:13
@auvipy

auvipy commented Jul 11, 2024

Copy link
Copy Markdown
Collaborator

we should merge this only in a major release

@stale

stale Bot commented Apr 26, 2025

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Apr 26, 2025
@ulgens

ulgens commented Apr 27, 2025

Copy link
Copy Markdown
Contributor

The fix seems still relevant.

@auvipy auvipy removed the stale label Apr 27, 2025

@auvipy auvipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if this will be accepted or not, but if it is accepted, fixing the merge conflict should be enough

@sevdog

sevdog commented Apr 28, 2025

Copy link
Copy Markdown
Contributor Author

Conflict revolved.

@peterthomassen peterthomassen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for resolving the conflict; some more comments here!

Comment thread docs/api-guide/fields.md Outdated
Comment thread rest_framework/__init__.py Outdated
Comment thread rest_framework/fields.py Outdated
Comment thread tests/test_fields.py Outdated
Comment thread tests/test_fields.py Outdated
Comment thread tests/test_fields.py Outdated
Comment thread docs/api-guide/fields.md Outdated
auvipy and others added 2 commits April 29, 2025 12:28
Co-authored-by: Bruno Alla <browniebroke@users.noreply.github.com>
Co-authored-by: Bruno Alla <browniebroke@users.noreply.github.com>
@stale

stale Bot commented Jul 19, 2025

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Jul 19, 2025
@pietzschke

Copy link
Copy Markdown

Can we have stale flag removed? We still need this @auvipy

@stale stale Bot removed the stale label Jul 21, 2025
@sevdog

sevdog commented Jul 22, 2025

Copy link
Copy Markdown
Contributor Author

I am still waiting to get a response in this thread.

I was also thinking of re-creating the branch and the PR to have less noise and a cleaner git history.

@sevdog

sevdog commented Aug 7, 2025

Copy link
Copy Markdown
Contributor Author

I have updated the code and docs according to latest comments.

The validation check for format is performed in both __init__ and to_representation methods because it may be altered by the user during the lifecycle of the field or the initial config could be wrong. I have not put a check also in the settings item just to not have the same logic in 3 different points (2 are more than enough and those are already safe), it could be added with a minimal effor to raise also an ImproperlyConfigured.

Comment thread rest_framework/fields.py
Comment thread rest_framework/fields.py Outdated

@peterthomassen peterthomassen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sorry, overlooked something.

Comment thread docs/api-guide/settings.md Outdated
Comment thread docs/api-guide/settings.md Outdated

@peterthomassen peterthomassen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

almost! 🙃

Comment thread docs/api-guide/settings.md Outdated

@peterthomassen peterthomassen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm!

@browniebroke browniebroke added this to the 3.17 milestone Aug 8, 2025
Comment thread rest_framework/fields.py
Comment thread rest_framework/fields.py
Comment thread rest_framework/fields.py
Comment thread rest_framework/fields.py

@browniebroke browniebroke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's keep it as is

@peterthomassen peterthomassen merged commit c8b6d3d into encode:master Aug 12, 2025
7 checks passed
@auvipy

auvipy commented Aug 12, 2025

Copy link
Copy Markdown
Collaborator

thanks for the patience!

@browniebroke browniebroke changed the title DurationField output format Add ability to specify output format for DurationField Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants