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

Process existing ics files from URL #71

Closed
wants to merge 3 commits into from

Conversation

melissawm
Copy link
Member

This PR adds functionality to process existing .ics files from a URL and create a compatible calendar. (The goal of this PR is to be able to show the result in scientific-python.org)

Addresses scientific-python/scientific-python.org#246

Feedback is welcome!

@melissawm melissawm added the type: Enhancement New feature or request label Oct 21, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2023

Codecov Report

Attention: Patch coverage is 75.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.96%. Comparing base (2dd77b4) to head (e9b03d1).
Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
yaml2ics.py 75.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #71      +/-   ##
==========================================
- Coverage   95.19%   91.96%   -3.23%     
==========================================
  Files           1        1              
  Lines         104      112       +8     
==========================================
+ Hits           99      103       +4     
- Misses          5        9       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@tupui tupui left a comment

Choose a reason for hiding this comment

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

Thanks Melissa! I only have a suggestion to use stdlib. The rest is maybe some doc and I suppose you were waiting for some feedback before adding a test?

@@ -24,6 +24,7 @@ dependencies = [
"ics == 0.8.0.dev0",
"python-dateutil >= 2.8",
"pyyaml >= 6",
"requests",
Copy link
Member

Choose a reason for hiding this comment

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

I would not add an extra dependency to requests or another HTTP library as the needs are very limited. urllib.request should be more than enough for that.

@@ -146,7 +147,7 @@ def events_to_calendar(events: list) -> str:
return cal


def files_to_events(files: list) -> (ics.Calendar, str):
def files_to_events(files: list) -> (ics.Calendar, str, bool):
Copy link
Member

Choose a reason for hiding this comment

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

Can we add some doc about that somewhere? The other parameters/return values were sort of ok to guess but this is less so to me.

@melissawm
Copy link
Member Author

Exactly, just checking if this was reasonable. Thanks for the suggestions - I'll fix them up and add the tests 😁

@stefanv
Copy link
Member

stefanv commented Oct 26, 2023

Given that yaml2ics turns yaml into ICS, and these calendars are already in ICS format, I wonder if it wouldn't make sense to rather modify the process on the scientific-python.org site instead. I.e., have a small Python script that calls yaml2ics for files without, say, an external-ics: field, otherwise fetch the ICS and put it where it needs to go.

I suggest this, because I think this functionality is unlikely to be required by users of this library, but is obviously already required on scientific-python.org!

@melissawm
Copy link
Member Author

That makes sense too - I was wondering what the best approach would be given that we might want to keep track of these calendars in one place (right now this would mean the folder where all yaml files are in scientific-python.org).

@rkdarst
Copy link
Contributor

rkdarst commented Nov 6, 2023

I'm at least somewhat interested in this idea!

It's a good question if there should be something else that merges ICS feeds after yaml2ics does its job. If someone makes template commands for that, please paste here!

@stefanv
Copy link
Member

stefanv commented Nov 6, 2023

Hi Richard, please see the pull request linked above for an example.

@stefanv
Copy link
Member

stefanv commented Sep 2, 2024

Please see scientific-python/scientific-python.org#519 (comment)

I think, unfortunately, we are not able to provide this functionality.

@stefanv stefanv closed this Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants