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

Code example in README #410

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

TobyBoyne
Copy link
Collaborator

Closes #409

Adds some examples of BoFire code (from the getting started notebook) to the README.

Copy link
Contributor

@bertiqwerty bertiqwerty left a comment

Choose a reason for hiding this comment

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

Thanks, Toby!

Copy link
Contributor

@bertiqwerty bertiqwerty left a comment

Choose a reason for hiding this comment

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

I just saw that one missing import makes the test fail. Further, you could put all imports to the top.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Behrang Shafei <50267830+bertiqwerty@users.noreply.github.com>
@jduerholt
Copy link
Contributor

Test is failing due to a new scipy release from today. I try to figure it out this evening. Thanks @TobyBoyne!

@jduerholt
Copy link
Contributor

scipy.integrate.simps was renamed to scipy.integrate.simpson in todays stable 1.13 release.

@TobyBoyne
Copy link
Collaborator Author

TobyBoyne commented Jun 25, 2024

scipy.integrate.simps was renamed to scipy.integrate.simpson in todays stable 1.13 release.

Thank you for investigating this @jduerholt! Will you be fixing this in the main branch? If so, please let me know once you have, and I will merge the changes.

@jduerholt
Copy link
Contributor

Hi Toby, the fix is now available in Main. Best, Johannes

@TobyBoyne
Copy link
Collaborator Author

@bertiqwerty The fix has been merged, and this is now ready to close (if you're happy with my comment about moving all imports to the top, copied below)

Re: imports, I think it's clearer if the imports are in their own cells (in the same way as the Getting Started notebook), so that it is clearer which module relates to what functionality. It also might be intimidating to have 7 lines of import statements at the very top.
If you feel strongly about this, happy to change it!

@bertiqwerty
Copy link
Contributor

Thanks Toby!

@bertiqwerty bertiqwerty merged commit 6ca1586 into experimental-design:main Jun 25, 2024
10 checks passed
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.

Add code examples to README
3 participants