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

Rename equivalent sources classes #255

Merged
merged 24 commits into from
Oct 19, 2021
Merged

Rename equivalent sources classes #255

merged 24 commits into from
Oct 19, 2021

Conversation

santisoler
Copy link
Member

@santisoler santisoler commented Oct 18, 2021

Rename EQLHarmonic and EQLHarmonicSpherical classes to EquivalentSources
and EquivalentSourcesSph, respectively. Refactor files organization: rename
equivalent_layer directory to equivalent_sources and the sources files to
cartesian.py and spherical.py. Split the file with the tests function by
coordinate system: Cartesian and spherical. Rename some tests functions. Add
new child classes EQLHarmonic and EQLHarmonicSph for backwards
compatibility that raise a FutureWarning when initialized. Update gallery
examples and documentation pages to use "equivalent sources" instead of
"equivalent layer". Mention "equivalent layer" in one gallery example though.
Rename eql variables in examples and tests with eqs.

TODO:

  • Update examples
  • Update "Equivalent Layer" in docs
  • Rename eql variables in the examples and tests
  • Mention "equivalent layer" somewhere in the docs

Fixes #250

Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍 I like the new names. Just need to double check that all instances of “layer” have been replaced.

@santisoler
Copy link
Member Author

Thanks @leouieda for the review. Yes, I need to double check if I've changed "layer" everywhere else (besides the docs, which need some work).

I do have some doubts: we used to use eql for naming the equivalent layer instances on examples, would be better to rename then to equivalent_sources or eq_sources?

One more thing: I was thinking to leave the "equivalent layer" name somewhere in the docs so it could be indexed by search engines, just in case someone is looking for an "equivalent layer" implementation instead of an "equivalent sources" one.

@leouieda
Copy link
Member

That's a good point about the search terms. It may be good to leave it in a tutorial or the docstring as something like "equivalent sources (a.k.a. equivalent layer)" or something like that.

I do have some doubts: we used to use eql for naming the equivalent layer instances on examples, would be better to rename then to equivalent_sources or eq_sources?

How about eqs since we call a lot of methods with lots of arguments on it?

@santisoler santisoler added this to the v0.3 milestone Oct 19, 2021
@santisoler santisoler merged commit 28a12e1 into master Oct 19, 2021
@santisoler santisoler deleted the rename-eqls branch October 19, 2021 17:22
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.

Rename EQLHarmonic and EQLHarmonicSpherical classes into something more meaningful
2 participants