Skip to content

Conversation

@jobovy
Copy link
Contributor

@jobovy jobovy commented Jun 3, 2022

Here's a version of the solution I mentioned in #76 (comment), adding a pyproject.toml file to have numpy available in the setup.py file.

I also made a few other changes that I think improve the package, but that aren't necessary (the only necessary part is the addition of the pyproject.toml file):

  • Add scipy to the install_requires argument, because I think scipy, like numpy, can now be reliably installed by pip (I've had it as a dependency for a long time in galpy and nobody complains...)
  • Just assume that numpy is available in setup.py, so remove all code to deal with the case where it isn't
  • I changed your workflow to not use miniconda, but instead just use pip to install things. This is much faster, ~1 min. vs. 2.5 min. (pip installs of numpy and scipy are very fast, because pip installs binary wheels). I also moved installs to the relevant section (e.g., flake8 in the linting step, pytest in the test step).
  • I also changed the workflow to run for all branches (that way I could run it in my branch...).

@esheldon
Copy link
Owner

esheldon commented Jan 9, 2023

Sorry I let this one slip, can you please update for the newest version?

@jobovy
Copy link
Contributor Author

jobovy commented Jan 9, 2023

I merged the latest master branch, can you approve the workflow runs?

@jobovy
Copy link
Contributor Author

jobovy commented Jan 9, 2023

Okay, the tests pass in my fork, so everything's probably fine. I accidentally added some generated files and deleted them again, but you might want to squash and merge this to avoid including that in the history. You might also want to add python 3.10 and 3.11 to the test suite, as a lot of people are probably on 3.10 by now.

@esheldon
Copy link
Owner

Thanks Jo.

@esheldon esheldon merged commit 85cef99 into esheldon:master Jan 10, 2023
@jobovy jobovy deleted the add-pyproject.toml branch January 10, 2023 14:59
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