Skip to content

Fix timelex#122

Merged
pganssle merged 2 commits into
dateutil:masterfrom
ms-boom:fix_timelex
Oct 31, 2015
Merged

Fix timelex#122
pganssle merged 2 commits into
dateutil:masterfrom
ms-boom:fix_timelex

Conversation

@ms-boom

@ms-boom ms-boom commented Sep 10, 2015

Copy link
Copy Markdown
Contributor

fix wrong tokens on unicode string

@pganssle

Copy link
Copy Markdown
Member

I like where this is going, but I think we're actually losing some flexibility by enforcing isalpha() and isdigit(), etc. Right now, you can subclass _timelex() and override the relevant character sets, but in your version, that's no longer possible.

That said, I think that this is the right default approach. Today I'll make a PR against your branch with a suggested improvement.

@pganssle

Copy link
Copy Markdown
Member

@jarondl This seems like a major change - any thoughts on the matter?

See also my PR at ms-boom/dateutil#1

@pganssle

Copy link
Copy Markdown
Member

I'm going to go ahead and merge this. We can always undo it later if it causes some significant problems.

pganssle added a commit that referenced this pull request Oct 31, 2015
@pganssle
pganssle merged commit 9e54f8d into dateutil:master Oct 31, 2015
@jarondl

jarondl commented Nov 1, 2015

Copy link
Copy Markdown
Member

Hi @pganssle, and @ms-boom,
a post-factum +1 from me. It is much better than what we had.

aschatten pushed a commit to aschatten/dateutil that referenced this pull request Nov 16, 2015
@pganssle pganssle mentioned this pull request Feb 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants