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

Make Skyfield, Astronomy optional #660

Closed
jrd opened this issue Jun 29, 2021 · 8 comments
Closed

Make Skyfield, Astronomy optional #660

jrd opened this issue Jun 29, 2021 · 8 comments

Comments

@jrd
Copy link
Contributor

jrd commented Jun 29, 2021

astronomy module might interest some people I’m sure, but the skyfield dependency will pull numpy which is huge (and maybe complex to build on some environment).

So is it possible to have a workalendar package with a [astronomy] extra that will pull skyfield and skyfield-data?
That way, people who also needs the astronomy module will request workalendar[astronomy] and others just workalendar.

This is, of course, if it’s possible.
What do you think?

@ewjoachim
Copy link
Contributor

ewjoachim commented Jun 30, 2021

One other possible solution could be to have a different package (in the same codebase) that uses those libs to generate the astronomical information and that we can run once a year or once a release with everything, and have workalendar use these information but not require the libs themselves.

That would take advantage of the fact it's very predictible for long term. We could generate 50y worth of astromonical data and it's likely not to change a lot in the next 50 years.

@jrd
Copy link
Contributor Author

jrd commented Jul 1, 2021

Yes that seems to be a good idea.

@ewjoachim
Copy link
Contributor

(Would that be compatible with our dependencies' licences ? I think so but I don't know)

@jrd
Copy link
Contributor Author

jrd commented Jul 12, 2021

How can I help getting this done?

@ewjoachim
Copy link
Contributor

ewjoachim commented Jul 12, 2021

I would have like to get an opinion from @brunobord before spending time on this. He's the maintainer and he's the architect for this lib :)

If he says 👍 then I believe you can start working on a PR.
What needs to be done is to create an intermediary format, easily writtable on disk (I'd say JSON) that lets you calculate all the working/non-working days that depends on astronomy elements without actually calling the astronomy-related libs (so maybe this will be for example a list of all relevant calculation results for the 50 next years). This probably means the first thing to do is audit the entire lib and find how the astronomy-related libs are called.

Then once this is done, we need a script that will generate those JSON files, and we'll need a bit of "packaging magic" so that it's easy to run the script with the astronomy libs, and we can safely remove them from the runtime workalendar (extras_require could be a lead, but I'd prefer if we could remove those entirely from the lib. That being said, may a simple file requirements.astronomy.txt would suffice).

Does this make sense ?

@jrd
Copy link
Contributor Author

jrd commented Jul 13, 2021

Yes totally. I wait for @brunobord opinion. Thanks for the quick answer.

@brunobord
Copy link
Member

Made a 16.0.0rc1 release ; will have to make a few tests tomorrow, we're getting closer.

brunobord added a commit that referenced this issue Sep 16, 2021
**Warning**: Important changes in the runtime requirements. Please have a look at the README for more information.

**New calendar**

- New calendar: Added Philippines calendar by @micodls (#396)

**Internal changes**

- Remove `skyfield` dependency, added `[astronomy]` as extra dependency (#660).
- Replace `pyCalverter` with `convertdate` (#536).
- Remove unused `JalaliMixin`
- Replace `pkg_resources` with `importlib_metadata` to fetch the version number in `__init__.py` (#657)
- Added new badges (pypi, conda, license) and installation instructions (pip, conda) to readme file @sugatoray (#673).
- Added the "Workalendar maintainers" in the LICENSE file.
- Changed the maintainer email.
@brunobord brunobord mentioned this issue Sep 16, 2021
14 tasks
@brunobord
Copy link
Member

The latest release (16.0.0) includes your tremendous work. Thank you a million for your time and your patience. You rock!

Happy upgrading!

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

No branches or pull requests

3 participants