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 docstring for Dates.adjust #52914

Merged
merged 7 commits into from
Jan 27, 2024
Merged

Conversation

jam-khan
Copy link
Contributor

This is a PR for the issue #52725.

Thanks!

@jariji
Copy link
Contributor

jariji commented Jan 15, 2024

I'm not actually sure this function is supposed to be public. #52746 @quinnj do you know?

@stevengj
Copy link
Member

You also need to update the docstring test for the Dates module, similar to #52913 (comment)

@stevengj
Copy link
Member

Regardless of whether it should be exported/public or not, it doesn't hurt to have a docstring.

@@ -204,6 +226,27 @@ function adjust(df::DateFunction, start, step, limit)
throw(ArgumentError("Adjustment limit reached: $limit iterations"))
end

"""
adjust(df::DateFunction, start) -> TimeType
Copy link
Member

@stevengj stevengj Jan 15, 2024

Choose a reason for hiding this comment

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

Better to have a single docstring, i.e. merge these two.

We may not want to document the DateFunction method, actually, since DateFunction is not public.

Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
Comment on lines 199 to 220

"""
adjust(df::DateFunction, start, step, limit) -> TimeType

Adjusts the date in `start` until `f::Function` in `df::DateFunction` returns `true`.
The step size dictates change in `start` on every iteration. If `limit` iterations occur,
then an [`ArgumentError`](@ref) is thrown.

# Examples
```jldoctest
julia> adjust(date -> month(date) == 10, Date(2022, 1, 1), step=Month(3), limit=10)
2022-10-01

julia> adjust(date -> year(date) == 2025, Date(2022, 1, 1), step=Year(1), limit=4)
2025-01-01

julia> adjust(date -> day(date) == 15, Date(2022, 1, 1); step=Year(1), limit=3)
ERROR: ArgumentError: Adjustment limit reached: 3 iterations
Stacktrace:
[...]
```
"""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""
adjust(df::DateFunction, start, step, limit) -> TimeType
Adjusts the date in `start` until `f::Function` in `df::DateFunction` returns `true`.
The step size dictates change in `start` on every iteration. If `limit` iterations occur,
then an [`ArgumentError`](@ref) is thrown.
# Examples
```jldoctest
julia> adjust(date -> month(date) == 10, Date(2022, 1, 1), step=Month(3), limit=10)
2022-10-01
julia> adjust(date -> year(date) == 2025, Date(2022, 1, 1), step=Year(1), limit=4)
2025-01-01
julia> adjust(date -> day(date) == 15, Date(2022, 1, 1); step=Year(1), limit=3)
ERROR: ArgumentError: Adjustment limit reached: 3 iterations
Stacktrace:
[...]
```
"""

Only document the Function interface below, since DateFunction is private.

@LilithHafner LilithHafner added the merge me PR is reviewed. Merge when all tests are passing label Jan 27, 2024
@LilithHafner LilithHafner changed the title Docstrings added for adjust in Dates in stdlib Add docstring for Dates.adjust Jan 27, 2024
@LilithHafner LilithHafner added the dates Dates, times, and the Dates stdlib module label Jan 27, 2024
@LilithHafner LilithHafner merged commit 20085f4 into JuliaLang:master Jan 27, 2024
8 of 10 checks passed
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Jan 27, 2024
@vtjnash
Copy link
Member

vtjnash commented Jan 27, 2024

It appears that either this or #53027 needs to be reverted, due to a merge conflict between them that is breaking CI. Which is better to revert and reland?

@LilithHafner
Copy link
Member

The CI on master after they both merged looks green-ish to me:

https://buildkite.com/julialang/julia-master/builds/32740

@giordano
Copy link
Contributor

doctest job is very much red in the build you linked.

@LilithHafner
Copy link
Member

Thanks! I'm so used to red that I instinctively ignored the fact that a single check failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dates Dates, times, and the Dates stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants