Skip to content

Refactor _parse_numeric_token out from the big parsing loop#490

Merged
pganssle merged 5 commits into
dateutil:masterfrom
jbrockmendel:tokenization
Nov 6, 2017
Merged

Refactor _parse_numeric_token out from the big parsing loop#490
pganssle merged 5 commits into
dateutil:masterfrom
jbrockmendel:tokenization

Conversation

@jbrockmendel

Copy link
Copy Markdown
Contributor

Cut/paste/de-indent.

@pganssle

pganssle commented Nov 2, 2017

Copy link
Copy Markdown
Member

I'm not totally convinced of the point of this. Now instead of a huge loop body you have a huge function body? Is that really an improvement?

@jbrockmendel

Copy link
Copy Markdown
Contributor Author

The idea is that it would be really helpful if each step in the loop was just one function call, so we could step through it interactively. Instead of doing that directly, this is just taking one step in that direction by separating out a big chunk of it.

We've got a few different threads going in which we're not 100% on the same page. Let's put this on the backburner until we have some concensus-momentum going.

@pganssle

pganssle commented Nov 5, 2017

Copy link
Copy Markdown
Member

@jbrockmendel Hm... I think I may have screwed this up by doing a rebase? There was a merge conflict that I fixed and did a force push, and it broke the dBY test. I'm not sure why. Maybe best if you force push the original version back up here, then do the rebase again on your side and fix the merge conflicts.

@pganssle
pganssle merged commit 70b0887 into dateutil:master Nov 6, 2017
@pganssle pganssle mentioned this pull request Mar 11, 2018
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.

2 participants