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 Setup for Enhanced Compatibility Across Python 3.8, 3.9, 3.10, 3.11, 3.12 #107

Merged
merged 22 commits into from
Apr 22, 2024

Conversation

ben9809
Copy link
Contributor

@ben9809 ben9809 commented Apr 18, 2024

Purpose

This pull request aims to set up Poetry for dependency management in the project. The integration of Poetry is intended to streamline our development process by simplifying dependency management and packaging.

Approach

The integration of Poetry replaces the previous dependency management system. By utilizing Poetry, we can define project dependencies in a more structured pyproject.toml file instead of the traditional requirements.txt. This change addresses the need for a more robust and easy-to-manage dependency configuration. The pyproject.toml file has been created with all the necessary dependencies. Some dependencies have been upgraded to a new version to ensure no conflict between each other, this triggered new warnings that lead to a refactor of some files.
A new workflow setup has been updated, to fully leverage poetry capabilities.

Tests for New Behavior

Major Changes

.coveragerc -> Enable parallel mode when executing coverage

.github/workflows/python-tests.yaml -> Cover all python versions from 3.8

.pylintrc -> Due to an upgrade of pylint to cover multiple Python versions, some pylint setting have been deprecated, so they have been removed from the configuration file

arbitragelab/__init__.py -> redefine imports to comply with the new warning generated by the new pylint version

arbitragelab/ml_approach/optics_dbscan_pairs_clustering.py -> To support Python 3.12 a new version of sklearn is needed. Since the class TSNE in the new version has different default parameters, it has been adapted to reflect the behavior of the previous versions. Here is the link: https://scikit-learn.org/stable/modules/generated/sklearn.manifold.TSNE.html

docs/source/conf.py -> logo fix

tests/test_neural_networks.py, tests/test_spread_modeling_helper.py -> adapt imports and 'Session configuration' to new tensorflow version compatible across multiple python versions

All the rest of the files changed have been subjected to minor changed mostly to comply with the new pylint version

Checklist

None

Learning

None

@ben9809 ben9809 changed the title Setup Poetry Poetry Setup for Enhanced Compatibility Across Python 3.8, 3.9, 3.10, 3.11, 3.12 Apr 21, 2024
Copy link

@mripani mripani left a comment

Choose a reason for hiding this comment

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

I managed to install the project with poetry, great! I tried to manually install the cvxpy package but I got no enough memory for it. I left a suggestion about the requirements.txt file that is not used anymore.

Copy link

Choose a reason for hiding this comment

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

I understand that this file is not used anymore in the new workflow. The dependencies are handled directly with poetry, wouldn't it be better to remove this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this file should be removed.
I also think we should remove .bumpversion.cfg

Copy link

Choose a reason for hiding this comment

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

I agree, it is also a good opportunity to add a release workflow.

Copy link
Member

Choose a reason for hiding this comment

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

Why would we remove bumpversion?

It is used to easily update the version number in several locations, before we do a release.

{ include = "arbitragelab" }
]

exclude = ["contrib", "docs", "tests"]
Copy link

Choose a reason for hiding this comment

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

Is it contrib still in the repository root?

Copy link
Contributor Author

@ben9809 ben9809 Apr 22, 2024

Choose a reason for hiding this comment

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

It's not in the repo, I forgot to remove it.

@ben9809
Copy link
Contributor Author

ben9809 commented Apr 22, 2024

@mripani Unfortunately the package cvxpy on Python 3.8 and 3.9 on macOS gives some problems, I installed it manually with the command conda install -c conda-forge cvxpy.

Copy link
Member

@Jackal08 Jackal08 left a comment

Choose a reason for hiding this comment

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

This was a large effort!

Thank you for this great contribution! You are on fire!

Copy link
Member

Choose a reason for hiding this comment

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

Why would we remove bumpversion?

It is used to easily update the version number in several locations, before we do a release.

@Jackal08 Jackal08 merged commit 4454610 into hudson-and-thames:develop Apr 22, 2024
11 checks passed
@Jackal08
Copy link
Member

Thank you so very much!

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