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

Restructure #227

Merged
merged 234 commits into from
Nov 17, 2020
Merged

Restructure #227

merged 234 commits into from
Nov 17, 2020

Conversation

jcapriot
Copy link
Member

There are a lot of changes in here for deprecations. I will generate more details on our reasoning for this soon.
The main purpose is to move everything to a pep8 style of naming convention for modules, classes, methods, functions, and arguments.
These deprecations will likely stick around for a while (Current estimate is discretize v 1.0.0). So there should be plenty of time for users to update.
One big change is that the items such as vnC, or vnN now return tuples (immutable objects) as it is unsafe to actually change these. This was not guarded against previously. The downstream implication is that code that relied on these items being numpy.ndarrays will break. (i.e. things like mesh.vnC + 1 will no longer work, instead something like [n+1 for n in mesh.vnC]) would likely be necessary.

I have also separated out the plot commands even further into a Matplotlib mixin style class for each mesh.

@jcapriot jcapriot merged commit 968128b into simpeg:master Nov 17, 2020
@jcapriot jcapriot deleted the restructure branch November 17, 2020 03:26
@prisae
Copy link
Member

prisae commented Nov 18, 2020

Just a note for the future. I think we should avoid such massive PRs, and try to stick to one issue per PR. Hence this one would have been better, IMHO,

  • 1 PR for name changes,
  • 1 PR for azure pipelines,
  • 1 PR for dropping Python 2.7 stuff,
  • 1 PR for matplotlib fixes,
  • etc,
    and each of this PR should be squashed at the end, giving each commit a sensible commit message. I am trying to figure out what is causing my failures now due to this PR, and this is sort of difficult in a 234 commit PR with many cryptic commit messages... ;-)

Not that I would follow these guidelines thoroughly, I blame myself here too, it is more something we should probably strive for...

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.

6 participants