basil96 / civilite Goto Github PK

Current requirements*.txt list exact package versions. Try using >= for all dependencies so that things march forward in future releases.

The README file is very helpful describing the dependencies, but typically a requirements.txt file or setup.py file carry the dependencies. Currently, requirements.txt is more appropriate.

see here

The current astral.py module we use here is a local copy from the official astral package, with custom locations added to the _LOCATION_INFO string. See if it's possible to add a custom location from client side so we would eliminate astral.py from this project and just use the official package via pip so that future updates to astral go smoothly.

• Create a civilite.Configuration class.

• Storage schema:

  • use JSON for the config file.
  • Store the file as configuration.json in the user's home dir: os.path.expanduser('~').
  • Windows: .civilite subdirectory.
  • Linux: ~/.local/share/civilite directory.

• API: TBD. A simple dictionary based directly on JSON might suffice.

• As a start, move getCurrentSchedule() function out of schedule.py and into configuration.json.

Once issue #14 is completed, we should update code to the standards.

Discussion in #27 revealed that our scheduling logic is untested with regard to timezones. We need at least these test cases:

1. WeeklySchedule instance provided with a tzinfo that matches a provided location.

   • expectation: getCivilTwilight should return the appropriate datetime for the location on a given date in the provided tzinfo timezone.

2. WeeklySchedule instance provided with a location and tzinfo=None.

   • expectation: getCivilTwilight should return the appropriate datetime for the provided location on a given date but in the timezone of the caller's OS.

3. WeeklySchedule instance provided with a tzinfo that is different from a provided location.

   • expectation: getCivilTwilight should return the appropriate datetime for the provided location on a given date but in the provided tzinfo timezone.

4. WeeklySchedule instance provided with a location and no tzinfo argument.

   • expectation: getCivilTwilight should return the appropriate datetime for the provided location on a given date but in the UTC timezone.

Choose a few sample dates, locations, and twilight times from timeanddate.com, no fancy web scraping needed.

Bonus: use dates/times/locations where DST is involved. For example, try a location where DST is not practiced (Arizona?) while providing a tzinfo where DST is in effect on the given day.

These functions in schedule.py make more sense as instance methods of WeeklySchedule class:

• getEventType()
• createEvents()

Move them in there after #15 is merged.

A parameter is passed, but the global is used instead.

I've never done this before, so this is a science project.
I think this involves a setup.py script and a manifest file, among other things, that turn this project into a distributable package.
The advantages are many; you get a properly installed package, you're forced to run tests against the installed code instead of a temporarily patched sys.path, which should promote catching any issues early.

Change the default to False, make callers request it explicitly.

A bad merge or something else removed the schedule.py docstring and version

We need regression test(s) for the current state of the schedule generator, at least just for the current year's events.

.gitignore should include the common output files from this module
also, please add .vscode/ for those of us editing with .vscode

Create and add a .pylintrc for existing coding standards

Move the outputSunsets() function and its module-level client code to a new file scripts/make_calendar_csv.py. Leave the functionality unchanged, i.e., it should still create a sunsets.csv file as before.

The goal is to make sunsets.py a library module, not a top-level script.

I wonder how we fix this.

our insights/dependencies points to

https://github.com/peplin/astral

instead of

https://github.com/sffjunkie/astral

when I pip installed, it picked the right one. I wonder if this is something we can do in requirements.txt or if we need to report it to GitHub.

I believe the intent for tzinfo in ScheduleObserver is to be an instance attribute, not a shared one. The subtle difference can lead to hard-to-track bugs.

regular python standard (pep8), global vals should be UPPERCASE