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

[RFC] Split the times module into submodules #55

Open
GULPF opened this issue Jul 20, 2018 · 14 comments
Open

[RFC] Split the times module into submodules #55

GULPF opened this issue Jul 20, 2018 · 14 comments

Comments

@GULPF
Copy link
Member

GULPF commented Jul 20, 2018

The times module has grown a lot, and it's reaching a point where it would benefit from being split into submodules. Submodules have several advantages:

  • easier to maintain
  • easier for third party libraries that want to reimplement only parts of the times module
  • docs becomes more focused and less messy

I propose the following submodules:

  • std / times_core - core types (Time & Duration mostly) and related procs
  • std / times_calender - the calendar types (DateTime & TimeInterval mostly) and related procs
  • std / times_format - the procs for formatting and parsing DateTime
  • std / times_monotonic - monotonic time (like timers.nim in the compiler, not implemented yet).

It might be easier to merge times_core and times_calender into one, but I think it would be nice to keep them separated if possible. Feel free to bikeshed what submodules should exists and what they should be called.

Of course, the times module would still exists, but it would only import and export all the submodules (like the async module).

@GULPF GULPF changed the title [RFC] Split the times module into submodule [RFC] Split the times module into submodules Jul 20, 2018
@skilchen
Copy link

Thats a good idea, but i would prefer a split along the lines:

  • times_time
  • times_time_monotonic
  • times_datetime (wallclock time without any timezone support)
  • times_zone
  • times_zoneddatetime
  • times_calendar

parsing/formatting has to be specific to the submodules so it should be part of the respective submodule.

@GULPF
Copy link
Member Author

GULPF commented Jul 20, 2018

@skilchen What would times_calendar contain in your proposal? Also, I don't think we should use datetime to refer to a module without timezone information since we already use DateTime for the type that has timezone information. But having a separate module for the types that require timezone information makes sense.

parsing/formatting has to be specific to the submodules so it should be part of the respective submodule.

I disagree. Since all the types should use the same minilanguage it should be in it's own submodule.

@skilchen
Copy link

times_calendar would contain conversion routines between different calendars, such as gregorian, julian, modified julian, and what we like to support from the many calendars in use around the world.

@mratsim
Copy link
Collaborator

mratsim commented Jul 21, 2018

Can't we have:

import times # imports everything
import times/datetime # only datetime
import times/calendar # only calendar

@GULPF
Copy link
Member Author

GULPF commented Jul 21, 2018

@mratsim If that's possible then would prefer it to, although I think Araq wants new modules to go into the std namespace. What would the folder structure look like? I tried the following but it doesn't work:

/lib/pure/times.nim
/lib/pure/std/times/datetime.nim

Trying to import std / times / datetime results in Error: cannot open file: /nim/lib/pure/times.nim/datetime.

@Araq
Copy link
Member

Araq commented Jul 21, 2018

Trying to import std / times / datetime results in Error: cannot open file: /nim/lib/pure/times.nim/datetime.

I think this should work...

@mratsim
Copy link
Collaborator

mratsim commented Jul 21, 2018

I don't mind the std namespace it's just the underscore that are bugging me.

This is a perfect case to show the endorsed way of using submodules so let's use them.

@cooldome
Copy link
Member

cooldome commented Jul 23, 2018

I would like to be able to use Date separately from Time.
Something like:

std / date - date only
std / times - date time
std / times_format - the procs for formatting and parsing Date Time
std / times_monotonic - monotonic time (like timers.nim in the compiler, not implemented yet).

@Araq
Copy link
Member

Araq commented Jul 24, 2018

Just FYI I don't have any opinion (yet?). Will probably complain if the split results in more implementation complexity though.

@narimiran narimiran transferred this issue from nim-lang/Nim Jan 2, 2019
@konsumlamm
Copy link

Any update on this?

@Araq
Copy link
Member

Araq commented Feb 10, 2021

@konsumlamm I'm afraid no. But feel free to experiment.

@Varriount
Copy link

I'm against this change. Python has a similar architecture (with time and datetime modules) and it's always frustrating trying to remember which module contains which functionality.

@konsumlamm
Copy link

I'm against this change. Python has a similar architecture (with time and datetime modules) and it's always frustrating trying to remember which module contains which functionality.

FYI, std/times would still reexport everything, but if you only need specific functionality (e.g. the duration API), you can only import the specific submodule, to avoid paying the price of the full std/times module.

@Araq
Copy link
Member

Araq commented Jul 12, 2021

to avoid paying the price of the full std/times module.

And what is this price? Unused code gets optimized away and scope pollutions can be dealt with via from times import x, y, z (which maybe would work more effectively by my type-bounded ops proposal).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants