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

Add PySR integration test #461

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

MilesCranmer
Copy link
Contributor

Now that PySR has switched to PythonCall.jl, here is the promised integration test :)

I'm installing and running it in a separate virtualenv so as to not interfere with anything.

@cjdoris
Copy link
Collaborator

cjdoris commented Feb 18, 2024

This looks cool thanks! However I don't think integration tests are suitable for running on every commit in regular CI. Instead I think it would make sense to have a separate github workflow for integration tests which is run manually - and I'll ensure to run it before each release.

Are you familiar enough with github workflows to do this or would you rather I did it?

@MilesCranmer
Copy link
Contributor Author

Sounds good to me – just adding the workflow_dispatch key right?

Also would it be useful to have a daily/weekly cron job or something, to test against main? (but not so regular as to be a nuisance)

@cjdoris
Copy link
Collaborator

cjdoris commented Feb 18, 2024

Yeah a weekly cron job would also be fine.

@MilesCranmer
Copy link
Contributor Author

@cjdoris I have greatly simplified the integration test so I think it is now appropriate to run on each commit. In fact it takes less time than many of the other GitHub actions.

I think it would be very useful to have something like this as it tests more complex workflows for scenarios not probed by the unittests alone, such as giving an early indication of issues such as #537 (due to the PyNULL typo).

What do you think?

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.

2 participants