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

Changed internal rrule._byxxx variables back to tuples (now sorted). #54

Merged
merged 1 commit into from
Mar 5, 2015

Conversation

pganssle
Copy link
Member

@pganssle pganssle commented Mar 5, 2015

I decided that at least for now, it's best not to keep these as sets. Although we're under no particular API obligations to maintain the data types of internal variables, I don't think the speedup is particularly dramatic, and I know there's code out there relying on the fact that these are stored as slicable iterables (e.g. #50 and #24) and I'm afraid that having a consistent order might be important for some things as well.

If the data set is going to change, we can hold that off for rrule2, which will be rearranging the internal structure of rrule dramatically anyway.

This also fixes one bug in the __construct_byset method that I found (albeit one that is currently unreachable).

…They are still sets first, so duplicate input values are discarded.
pganssle added a commit that referenced this pull request Mar 5, 2015
Changed internal rrule._byxxx variables back to tuples (now sorted).
@pganssle pganssle merged commit 4a224c7 into master Mar 5, 2015
pganssle added a commit to pganssle/dateutil that referenced this pull request Mar 5, 2015
@pganssle pganssle deleted the tuple-byxxx branch March 5, 2015 17:44
aschatten pushed a commit to aschatten/dateutil that referenced this pull request Nov 16, 2015
Changed internal rrule._byxxx variables back to tuples (now sorted).
aschatten pushed a commit to aschatten/dateutil that referenced this pull request Nov 16, 2015
@pganssle pganssle added this to the 2.4.2 milestone Feb 25, 2016
@pganssle pganssle added the rrule label Feb 25, 2016
mgeubelle added a commit to odoo-dev/odoo that referenced this pull request Oct 28, 2016
Depending on the version of dateutil, rule._bynweekday can either be
a tuple or a set (see dateutil/dateutil#54), which,
in the case of a set, breaks the access by index (see related issue:
dateutil/dateutil#24).

By casting it into a list, we make sure that we can access [0] in both case.

Credits to jke ; closes opw-690761.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant