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

Remove fmt: prefix for date formats #260

Closed
pwalsh opened this issue May 10, 2016 · 15 comments
Closed

Remove fmt: prefix for date formats #260

pwalsh opened this issue May 10, 2016 · 15 comments

Comments

@pwalsh
Copy link
Member

@pwalsh pwalsh commented May 10, 2016

I added it to the spec as a way to handle date formatting... but it is pretty ugly and does not feel right. We can probably use constraints.pattern to achieve the same, and possibly document some patterns for common date formats.

@roll
Copy link
Member

@roll roll commented May 11, 2016

constraints.pattern 👍

@rufuspollock
Copy link
Contributor

@rufuspollock rufuspollock commented May 13, 2016

@pwalsh hmmm - i am not sure this is in contraints - basic parsing and checking constraints seems to me different things and though we could use constraints.pattern it would be a bit of an abuse. What about:

format: default | any | pattern
pattern: {pattern}

i.e. we introduce pattern attribute and a pattern option for format.

@pwalsh
Copy link
Member Author

@pwalsh pwalsh commented Jul 12, 2016

@rgrp i see. It might be a bit confusing to have both pattern and constraints.pattern though?

@roll @vitorbaptista @akariv @amercader any thoughts?

@roll
Copy link
Member

@roll roll commented Jul 12, 2016

Using constraints.pattern for both (special handling if format: pattern) could be nifty move.. But also confusing?

Or we need other word I suppose to replace pattern may be:

format: template
template: %Y-%m
@akariv
Copy link
Member

@akariv akariv commented Jul 12, 2016

We had the same issue with numeric values.
There we opted to have custom properties (decimalChar and groupChar iirc).
For the sake of consistency, we should do the same here and create a property called dateFormat (or similar) with the format string.

@roll
Copy link
Member

@roll roll commented Jul 12, 2016

But it will lead to dateFormat, timeFormat etc? For every type supporting current fmt:?

@akariv
Copy link
Member

@akariv akariv commented Jul 12, 2016

@roll yes - you could call it stringFormat then it would be the same for all date/time types

@roll roll added the backlog label Aug 8, 2016
@roll roll added this to the specs-v1 milestone Aug 8, 2016
@roll roll added priority and removed backlog labels Aug 8, 2016
@roll roll removed this from the specs-v1 milestone Aug 8, 2016
@Hypercubed
Copy link

@Hypercubed Hypercubed commented Aug 9, 2016

IMO, if I understand this issue correctly, it would be nice to format and pattern in the properties in the fields. Then it would be easy to then to treat format: "fmt:%Y" as an alias for:

"format": "pattern",
"pattern": "%Y"
@rufuspollock
Copy link
Contributor

@rufuspollock rufuspollock commented Aug 9, 2016

@Hypercubed good suggestion.

@pwalsh I'd like to make a choice here soon. I think I'm a definite -1 (or, at least, -0) on overloading constraints.pattern - i want to keep validation and parsing separate conceptually and practically.

This basically leaves us with this as the proposal:

format: default | any | pattern
pattern: {pattern}

We could rename pattern to stringFormat if we wanted (and easy on that one if anyone has views).

What do people think?

Aside

Worth thinking about use cases. In my experience with core datasets the major use cases were being able to handle plain year, and year month (i.e. YYYY and YYYY-MM) and occasionally quarters YYYY-QX. I wonder if we should add these as specific formats e.g.

format: default | year | year-month | any | pattern

This is something of an aside and should perhaps be another issue ...

@akariv
Copy link
Member

@akariv akariv commented Aug 9, 2016

Re the aside: I would say that quarter / month / year are actually
different types (along with time and date), as they refer to different time
resolutions than date (which basically points to a single calendar day).

On Tue, 9 Aug 2016 at 11:45 Rufus Pollock notifications@github.com wrote:

@Hypercubed https://github.com/Hypercubed good suggestion.

@pwalsh https://github.com/pwalsh I'd like to make a choice here soon.
I think I'm a definite -1 (or, at least, -0) on overloading
constraints.pattern - i want to keep validation and parsing separate
conceptually and practically.

This basically leaves us with this as the proposal:

format: default | any | pattern
pattern: {pattern}

We could rename pattern to stringFormat if we wanted (and easy on that
one if anyone has views).

What do people think?
Aside

Worth thinking about use cases. In my experience with core datasets the
major use cases were being able to handle plain year, and year month (i.e.
YYYY and YYYY-MM) and occasionally quarters YYYY-QX. I wonder if we
should add these as specific formats e.g.

format: default | year | year-month | any | pattern

This is something of an aside and should perhaps be another issue ...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#260 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAQMdW2PF5f_7Wphcrfb48iqm3pNAuZzks5qeD5AgaJpZM4Ia8lu
.

@jpmckinney
Copy link
Contributor

@jpmckinney jpmckinney commented Sep 27, 2016

In terms of naming, we're dealing with printf format strings, so would formatString make sense - or is there reason to believe it would be confusing?

Re: the aside and @akariv's comment, I think that issue is now in #105

Not really the place for this question, but what was the rationale for groupChar versus thousandsChar?

@akariv
Copy link
Member

@akariv akariv commented Sep 27, 2016

Re your second question - because not everyone uses it to separate
thousands.

On Tue, Sep 27, 2016, 16:06 James McKinney notifications@github.com wrote:

In terms of naming, we're dealing with printf format strings, so would
formatString make sense - or is there reason to believe it would be
confusing?

Not really the place for this question, but what was the rationale for
groupChar versus thousandsChar?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#260 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAQMdadAyNSCsQ-M_-bF6isSfQmnOIwiks5quRS7gaJpZM4Ia8lu
.

@jpmckinney
Copy link
Contributor

@jpmckinney jpmckinney commented Sep 27, 2016

What groups is it separating?

On Sep 27, 2016, at 10:27 AM, Adam Kariv notifications@github.com wrote:

Re your second question - because not everyone uses it to separate
thousands.

On Tue, Sep 27, 2016, 16:06 James McKinney notifications@github.com wrote:

In terms of naming, we're dealing with printf format strings, so would
formatString make sense - or is there reason to believe it would be
confusing?

Not really the place for this question, but what was the rationale for
groupChar versus thousandsChar?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#260 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAQMdadAyNSCsQ-M_-bF6isSfQmnOIwiks5quRS7gaJpZM4Ia8lu
.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@pwalsh
Copy link
Member Author

@pwalsh pwalsh commented Nov 10, 2016

@rgrp you are assigned here. I think we need to fix this. Do you want to do a PR, or shall we discuss further?

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

Successfully merging a pull request may close this issue.

None yet
7 participants