Skip to content

yaml2ics: Allow a "timezone" option within each event.#22

Merged
stefanv merged 2 commits into
scientific-python:mainfrom
rkdarst:per-event-timezone
Apr 29, 2022
Merged

yaml2ics: Allow a "timezone" option within each event.#22
stefanv merged 2 commits into
scientific-python:mainfrom
rkdarst:per-event-timezone

Conversation

@rkdarst

@rkdarst rkdarst commented Apr 28, 2022

Copy link
Copy Markdown
Contributor
  • I at first tried using the yaml datetime "+03:00" explicit offset.
    But, this created an output that worked in Google
    Calendar/Thunderbird, but not office 365 cloud. The problem seems
    to relate to the quoting of "UTC +03:00" in one place but not the
    other (and, as it turns out, it didn't work with either quotes
    there, or no quotes there!)
  • I had made the idea in this PR earlier, but didn't submit it because
    "less is more". But now this seems simpler (and more explicit
    somehow).
  • Changes are minimal and it just worked in one of my test casse.
    There is no effect if 'timezone' is not given in an event.
  • Review: decide if this is a good idea or not. Is there any simpler
    way to implement it?

@rkdarst

rkdarst commented Apr 28, 2022

Copy link
Copy Markdown
Contributor Author

Black needed to be bumped, see the top of the description at psf/black#2964

This needs to be done anyway, if this isn't accepted I will make a separate PR.

@stefanv stefanv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me; I only suggested a small tweak to the test.

Comment thread tests/test_calendar.py
assert "SUMMARY:New year" in cal_str
# It is possible that the ics-py TZID string changes, but hopefully this
# substring is fairly safe to test against.
assert "Europe/Helsinki:20220101T000000" in cal_str

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth asserting that New_York is not in there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I should have thought of that. Added.

rkdarst added 2 commits April 29, 2022 09:50
- I at first tried using the yaml datetime "+03:00" explicit offset.
  But, this created an output that worked in Google
  Calendar/Thunderbird, but not office 365 cloud.  The problem seems
  to relate to the quoting of `"UTC +03:00"` in one place but not the
  other (and, as it turns out, it didn't work with either quotes
  there, or no quotes there!)
- I had made the idea in this PR earlier, but didn't submit it because
  "less is more".  But now this seems simpler (and more explicit
  somehow).
- Changes are minimal and it just worked in one of my test casse.
  There is no effect if 'timezone' is not given in an event.
- Review: decide if this is a good idea or not.  Is there any simpler
  way to implement it?
@rkdarst rkdarst force-pushed the per-event-timezone branch from de7ee0e to 8f620db Compare April 29, 2022 06:51
@stefanv stefanv merged commit caaa5b5 into scientific-python:main Apr 29, 2022
@stefanv

stefanv commented Apr 29, 2022

Copy link
Copy Markdown
Member

Thanks!

@jarrodmillman jarrodmillman added this to the 0.1 milestone Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants