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

Enable conversion of CompoundPeriod to Period #40803

Merged
merged 1 commit into from
Aug 3, 2021

Conversation

sostock
Copy link
Contributor

@sostock sostock commented May 12, 2021

This enables convert(::Type{<:Period}, ::CompoundPeriod).

I did not add support for constructors, because I thought something like Minute(::CompoundPeriod) could be confusing (one could think that it extracts the Minute component from the CompoundPeriod).

This came up in this discourse thread.

@oxinabox
Copy link
Contributor

we should cross-reference this in Dates.toms and Dates.tons.

@sostock
Copy link
Contributor Author

sostock commented May 12, 2021

Sounds good to me, but Dates.toms and Dates.tons (and Dates.days) don’t even have docstrings. Are they considered part of the public API?

@dkarrasch dkarrasch added the dates Dates, times, and the Dates stdlib module label May 13, 2021
Copy link
Member

@quinnj quinnj left a comment

Choose a reason for hiding this comment

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

This sounds fine to me; but no, Dates.tons/Dates.toms shouldn't be considered part of the public API. They were meant to be internal and there are probably gotchas if people start using it unawares. We could probably expose something official in the future, but we'd want to put some explicit consideration around it.

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.

4 participants