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

Poetry packaging #458

Merged
merged 11 commits into from
Mar 6, 2024
Merged

Poetry packaging #458

merged 11 commits into from
Mar 6, 2024

Conversation

bwheelz36
Copy link
Collaborator

@bwheelz36 bwheelz36 commented Jan 31, 2024

In this PR, I have recast the packaging mechanism to user poetry. This brings us line with latest python recommendations around packaging using a pyproject.toml file. Note that setup.py which we were using is considered deprecated. I have also made a few other changes:

  • removed some other CI config files which aren't being used anymore
  • bumped our minimum supported python version to 3.8, both in our packaging and in our CI. Note that 3.7 is now deprecated.
  • bumped the minimum support scikit-learn version from >0.18 to >1.0

This PR paves the way for another change I would like to make: add a CI job for 'release' as described in #426. This would mean that when we release a package through GH releases, a version will also be automatically uploaded to PyPi.

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.91%. Comparing base (383cb29) to head (ee7642b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #458      +/-   ##
==========================================
- Coverage   98.70%   94.91%   -3.79%     
==========================================
  Files           8        9       +1     
  Lines         540      669     +129     
  Branches       90        0      -90     
==========================================
+ Hits          533      635     +102     
- Misses          3       34      +31     
+ Partials        4        0       -4     

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

@bwheelz36
Copy link
Collaborator Author

Update: also had to remove python3.8 from supported packages. Poetry is quite strict about compatibility, and there is no version of numpy which supports both 3.8 and 3.12. 3.8 is end of life in October this year, so I think this is ok.... (noting that previous PyPi packages will still be available and support older python versions).

Copy link
Member

@till-m till-m left a comment

Choose a reason for hiding this comment

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

LGTM (haven't had a look at the notebook changes though). I like that this cleans up some old, presumably unused files.

I would also ask you to wait with merging this until I'm done with the docstrings, if that's okay.

pyproject.toml Outdated Show resolved Hide resolved
@till-m
Copy link
Member

till-m commented Jan 31, 2024

At the risk of asking a potentially stupid question -- is it possible to host docs for multiple releases in parallel such that one can view the docs of a specific version instead of always viewing the most current one?

@till-m till-m mentioned this pull request Jan 31, 2024
3 tasks
@bwheelz36
Copy link
Collaborator Author

Hey @till-m

  • numpy depedency good catch, it was an accident to comment it out . Since numpy is a dependency of scipy we can get away with this, but it's bad practice to rely on another package to handle a dependency we use explicitly - I will fix.
  • multiple doc versions - this must be possible because readthedocs supports it. however, tbh I don't have the bandwidth to look at implementing it. For now, previous versions of the docs would always be available via the commit history on the gh-pages branch - which is clunky but functional I guess.
  • notebook changes - these are trivial, I was just re-running to make sure they still ran, no actual changes were made. I probably shouldn't have committed those changes sorry.
  • Yes, I won't merge until you give the ok :-)

bayes_opt/__init__.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
Copy link
Member

@till-m till-m left a comment

Choose a reason for hiding this comment

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

Didn't catch anything else. LGTM! :)

@bwheelz36 bwheelz36 merged commit cec177f into master Mar 6, 2024
7 of 8 checks passed
@bwheelz36 bwheelz36 deleted the poetry_packaging branch March 6, 2024 01:11
@till-m till-m mentioned this pull request Jul 8, 2024
1 task
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