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

BUG: linspace and logspace give incorrect results or crash with some inputs #544

Merged
merged 17 commits into from
Dec 18, 2024

Conversation

kyleaoman
Copy link
Contributor

The implementations of linspace and logspace are improved so that retstep can be used with linspace and logspace rejects inputs for start and stop with non-dimensionless units, and also correctly handles the base keyword argument with units.

Tests are updated to reflect changes.

Closes #542
Closes #543

@kyleaoman
Copy link
Contributor Author

pre-commit.ci autofix

2 similar comments
@kyleaoman
Copy link
Contributor Author

pre-commit.ci autofix

@kyleaoman
Copy link
Contributor Author

pre-commit.ci autofix

@neutrinoceros
Copy link
Member

Thank you so much for catching and fixing so many (of my) mistakes ! There's a merge conflict now that prevents merging, can you please update your branch with a rebase ?

@neutrinoceros neutrinoceros added the bug Something isn't working label Dec 18, 2024
@neutrinoceros neutrinoceros added this to the 3.0.4 milestone Dec 18, 2024
unyt/tests/test_array_functions.py Outdated Show resolved Hide resolved
unyt/tests/test_array_functions.py Outdated Show resolved Hide resolved
unyt/_array_functions.py Outdated Show resolved Hide resolved
unyt/_array_functions.py Outdated Show resolved Hide resolved
@neutrinoceros
Copy link
Member

pre-commit.ci autofix

@kyleaoman
Copy link
Contributor Author

pre-commit.ci autofix

@kyleaoman
Copy link
Contributor Author

I think that should address all of your review points, but let me know if there are any follow-ups or anything else.

And no problem for catching these bugs - I'm working on a big PR for swiftsimio adding support for all of the numpy functions (already did ufuncs a while ago) supported by unyt, so as I work through each one individually I'm finding a few things ;)

@kyleaoman
Copy link
Contributor Author

Incidentally, would be great to have a release of 3.0.4 before I get to the point of merging SWIFTSIM/swiftsimio#219. I'm getting close to handling all of the functions but expecting some refactoring after that, plus adding a few more tests and updating docs, so hoping to merge sometime in January - any chance of a release around that time?

@neutrinoceros
Copy link
Member

We're hoping to get unyt 3.0.4 out before 2025, though I can't make any promises.
Do you expect to have more patches for unyt after this one ?

@neutrinoceros neutrinoceros merged commit 3fb5a44 into yt-project:main Dec 18, 2024
9 checks passed
@kyleaoman
Copy link
Contributor Author

@neutrinoceros I don't know - if I find any more missing logic, then yes, but it's a matter of going through each function one at a time.

@neutrinoceros
Copy link
Member

I understand, I did go through the pain myself about 3 years ago and, not looking for excuses but I'm sure that trying to write ~300 functions in a short amount of time was at least part of the reason why so many of them have blatant bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: linspace with retstep fails BUG: logspace incorrectly accepts units
2 participants