search

fatiando / harmonica

Forward modeling, inversion, and processing gravity and magnetic data
https://www.fatiando.org/harmonica
BSD 3-Clause "New" or "Revised" License
208 stars 68 forks source link

Merge magnetic forward functions for prisms #448

Closed santisoler closed 7 months ago

santisoler commented 7 months ago

Merge the two forward functions for magnetic fields of rectangular prisms into a single one named prism_magnetic that includes a new field argument, mimicking the interface of the gravity forward functions. Make the prism_magnetic` function to ask magnetization vectors as a tuple of three arrays, where each one of these arrays. Update tests accordingly. Update the user guide page about forward modelling of prisms.

Relevant issues/PRs:

Fixes #446 and fixes #447

santisoler commented 7 months ago

@indiauppal, I think this is ready to be merged. Would you like to review it?

No worries if you don't, and there's no rush to do so. But I think it would be a nice experience for you!

If you agree, here is a GitHub guide on how to make a review: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

Let me know what do you think.

santisoler commented 7 months ago

Thanks @indiauppal for the review! Nice catches!

I've just pushed a few more changes: I filled the docstrings of the new private functions (which were a little bit scarse), and noticed that asking for the cast was hard to explain, so I changed it for the shape of the output arrays.

If CI doesn't fail, I'll merge this right away!

Thanks @santisoler for letting me review it!

No need to thank me, it's the other way around haha! BTW, don't feel you need to ask permission to review a PR that is interesting for you. Feel free to jump in and make the review!