-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Merge Dates.jl package into Base #7285
Conversation
🍰 |
Ok, got the travis fiddling figured out. For those unaware, this is the result of much massaging and polishing over the last 7 months or so. Much appreciation for input from @nolta and @StefanKarpinski. To be clear on what's included (and not included) here:
I'd love to get any feedback. You can also currently get all the functionality here by doing |
Woo hoo! Checking it out now. I like the (much discussed) idea of limited scope without TimeZones. I suppose we'll need some documentation? In particular, do you have any documentatio of the format strings supported for parsing and formatting? Finally, thanks @quinnj for your marathon efforts with dates. |
When it goes into base, what happens with packages that depend on it? Do we simply leave out references to it in REQUIRE and in the module itself? Awesome work! |
If this gets merged before 0.4 is released, I would guess that we could tag a version of Dates.jl that requires 0.4- that only is a empty shell without any code. That way anyone can REQUIRE Dates.jl in order to be compatible with 0.2 - 0.3. |
I'm deliberately not announcing anything publicly about Dates.jl so packages don't build up dependencies, but as @ivarne mentioned, there are easy ways to ensure nothing breaks. In the case of TimeSeries, I've made sure it'll be a painless transition (pretty just just replacing @aviks, yes, no documentation on purpose at the moment. I'm piecing it together currently (about 75% done), but I've also held off due to inevitable bike-shedding :). This hopefully makes it easier for people to check out and play around with (in particular for @StefanKarpinski, *cough, *cough...) and figure out if this is good to go. I tried to take a simple, yet flexible approach with the parsing/formatting code. The main idea is you can mark fields as delimited or fixed-width. With a delimited slot, you indicate what's in the field with the corresponding letter ('y'=>year, 'm'=>month, etc.) as well as the delimiter that the parser can expect to follow the field (i.e. "yy-mm-dd" will look for a '-' to indicate the end of the year field). With fixed width, you specify the width of the field by the number of letters, so "20140617" would have a format string of "yyyymmdd". There's also support for parsing arbitrary text as the month, which I think is pretty nifty. Check out the slew of examples here |
Just want to register my cheering for this PR. |
Amazing! |
Would love to see this one merged. Good job! |
I'm doing some work on the docs currently; quick question, probably best answered by doc gurus @pao or @nolta, I noticed many of the manual chapters prefix code blocks with .. doctest::
julia> BigInt(typemax(Int64)) + 1
9223372036854775808 What's the |
doctests turns the examples in the documentation into actual tests to make sure they stay up to date with Base. It works similar to Python's |
Ah, makes sense. So I guess it's smart enough to ignore comments, and @test (BigInt(typemax(Int64)) + 1) == 9223372036854775808 |
It looks like it uses a mess of regular expressions so there may be corner cases it does not handle properly. Best to manually run it over the doc / tests you are working on first to inspect the output. |
Very happy to see this. Thank you. |
Shoot. Definitely did not mean to merge this. Anyone can help to revert? |
Hmmm......I don't think anything actually got merged because I forced a blank local branch. Is there something to be reverted anyway? |
Hmmm.........here's what I did locally:
Isn't that the right way to over-write a PR? Why did this get merged then? |
Weird... Nothing on your author page. https://github.com/JuliaLang/julia/commits?author=quinnj Were you pushing a squashed commit? |
Yeah, I did a revamp of my local branch with new documentation and stuff, and so I was just going to overwrite the branch in this PR, but didn't end up committing my changes locally, so when I pushed, it was just an empty branch. That must have triggered github to close this somehow. |
The other tabs in the pull request imply that it has infinite commits, but no diff. The merge commit indicated above (5fc6e31) isn't actually a merge commit. I think you managed to run into some kind of GitHub bug. |
Weird. Sorry for the noise everyone. Will open a new PR soon (if the git gods feel so inclined...). |
Should this PR be closed in favour of the new one? |
@ViralBShah This PR is already closed (via the bizarro-merge). |
Fixes #3524.
Cross-reference: #5328, quinnj/Dates.jl#6, quinnj/Datetime.jl#12.