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

Dynamic testing matrix for combinations of python and numpy versions. #286

Merged
merged 43 commits into from
Dec 15, 2024

Conversation

wolph
Copy link
Contributor

@wolph wolph commented Dec 9, 2024

Partially custom written, partially GPT generated ;)

Let me know if it needs improvements. At least it finally works.

@jorenham jorenham self-requested a review December 9, 2024 13:10
@jorenham jorenham added the meta: CI Continuous integration label Dec 9, 2024
Copy link
Owner

@jorenham jorenham left a comment

Choose a reason for hiding this comment

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

Thanks for this! I left a couple of questions and suggestions, and many of them are actually about the same thing

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
scripts/generate_matrix.py Outdated Show resolved Hide resolved
scripts/generate_matrix.py Show resolved Hide resolved
scripts/generate_matrix.py Outdated Show resolved Hide resolved
scripts/generate_matrix.py Outdated Show resolved Hide resolved
scripts/generate_matrix.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@wolph wolph left a comment

Choose a reason for hiding this comment

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

Addressed all issues, but now it's broken

scripts/generate_matrix.py Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
return self.major, self.minor

@classmethod
def from_str(cls, version_str: str) -> "Version":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add it for now, but it might be useful when differentiating between regular versions and pre-releases

def from_str(cls, version_str: str) -> "Version":
if version_str.count(".") == 1:
version_str += ".0"
return cls(*map(int, version_str.strip().split(".")))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

True... I assumed we only wanted to test for final releases. I'm explicitly ignoring pre-releases.

scripts/generate_matrix.py Outdated Show resolved Hide resolved
scripts/generate_matrix.py Outdated Show resolved Hide resolved
@jorenham
Copy link
Owner

jorenham commented Dec 10, 2024

not sure if relevant here, but uv supports PEP 723 for defining dependecies in standalone scripts:
https://docs.astral.sh/uv/guides/scripts/#declaring-script-dependencies

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@jorenham

This comment was marked as resolved.

@jorenham jorenham mentioned this pull request Dec 10, 2024
3 tasks
Copy link
Owner

@jorenham jorenham left a comment

Choose a reason for hiding this comment

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

this matrix script is becoming rather awesome 😮

scripts/generate_matrix.py Outdated Show resolved Hide resolved
scripts/generate_matrix.py Outdated Show resolved Hide resolved
@wolph
Copy link
Contributor Author

wolph commented Dec 14, 2024

It's finally done! And it includes @miloth his tox implementation.

@jorenham jorenham self-requested a review December 14, 2024 14:29
tox.toml Outdated Show resolved Hide resolved
jorenham

This comment was marked as resolved.

@wolph
Copy link
Contributor Author

wolph commented Dec 15, 2024

it might be a good idea to explicitly print the numpy version from within the tox env, so that we can validate that it's actually using the correct one

I think it accidentally already does that:
image

@jorenham jorenham self-requested a review December 15, 2024 04:11
@jorenham jorenham merged commit 8793281 into jorenham:master Dec 15, 2024
25 checks passed
@jorenham
Copy link
Owner

Thanks @wolph and @miloth !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: CI Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants