lmas / opensimplex

This repo has been migrated to https://code.larus.se/lmas/opensimplex
MIT License
241 stars 29 forks source link

updated 3D noise array function #24

Closed Julian-Wyatt closed 2 years ago

Julian-Wyatt commented 2 years ago

given array coordinate inputs x,y,z, the function returns 3D array of shape (z,y,x) instead of 1D array.

lmas commented 2 years ago

Hi thank you for this. Would you be willing to update it further and match it with #25? I'm asking you because your PR was smaller of the two.

In short:

Julian-Wyatt commented 2 years ago

Hope those two commits fix the issues and help. I've used the other PR for consistency.

lmas commented 2 years ago

Whoa hold up now mister. I appreciate your time and effort, thanks, but now you've started to add in partial code that's unrelated to your initial PR.

It's not my intention to sound like an ungrateful muppet, I just want to help you get it right while keeping things clean for everyone else :)

It's way easier to work with small and clean PRs that contains just a single feature/update/whatever. So your initial PR was good. But it would had been better to open up a new PR for that third commit instead.


Edit: I also now realise that neither the linter nor the autoformatter will catch misaligned docstrings. This issue is on me though.

lmas commented 2 years ago

Can you revert the commit with the new fractal noise func and add it as a separate PR? And please run the new formatter on your code.

Julian-Wyatt commented 2 years ago

I've removed the ident from the doc string. I've reverted the final commit, adding the potential new feature. Instead added this as another PR, building on from this branch.

lmas commented 2 years ago

Thank you, looking better now! I don't have time during the week so I'll try to verify and merge this weekend 👍