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

rule._byweekday as set broke backward compatibility #24

Closed
pypingou opened this issue Jan 24, 2015 · 4 comments
Closed

rule._byweekday as set broke backward compatibility #24

pypingou opened this issue Jan 24, 2015 · 4 comments
Labels

Comments

@pypingou
Copy link

rule._byweekday used to be a list and moving it to a set broke backward compatibility.

For example, vobject no longer works with the most recent dateutil, backtrace available at:
http://jenkins.cloud.fedoraproject.org/job/fedocal/456/console

@ralphbean
Copy link

Attributes that begin with a _ are generally considered "private" and not part of a public API. So I wouldn't call it a break in backwards compatibility and instead call it a bug in that vobject library.

@pganssle
Copy link
Member

I agree with @ralphbean. vObject shouldn't be directly accessing private variables like that. Additionally, I'm fairly certain that rrule._byweekday used to be a tuple not a list.

@pypingou Is there no way to do what vObject is trying to do using the publicly exposed methods and functions? If not, maybe the solution is to add a public interface to some of these internal variables so that your use case is officially supported.

That said, it's not critical that these things be stored as sets, it just makes certain operations faster and it's the more natural way to store them (since they are unique and there's no reason for them to be ordered). If this is going to cause widespread compatibility problems, we can probably make them tuple again for now, and push all potentially breaking changes to the switch over to rrule2 described in Issue #15.

@pypingou
Copy link
Author

Thanks for your feedbacks, I think that in fact the much larger problem is the fact that vobject is pretty much dead upstream with no real project providing the same functionality (icalendar isn't complete iirc).

I will look if I can do something on the vobject side, which I think is the better long-term solution.

@jarondl
Copy link
Member

jarondl commented Jan 28, 2015

Well put @pganssle.
I am closing as wontfix.

relvant link: http://xkcd.com/1172/

@jarondl jarondl closed this as completed Jan 28, 2015
mgeubelle added a commit to odoo-dev/odoo that referenced this issue 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

No branches or pull requests

4 participants