ENH: improve API consistency for ProjectionPlot VS SlicePlot

[neutrinoceros]

PR Summary

This is another jab at the current API inconsistency between slice plots and projection plots.
This replaces #3450.

main branch

• SlicePlot is a "factory function" that dispatches between AxisAlignedSlicePlot and OffAxisSlicePlot classes
• ProjectionPlot is a class, but it only performs "axis aligned" projections, and we have OffAxisProjectionPlot but no AxisAlignedProjectionPlot

this work

• SlicePlot becomes a class, but does the same exact thing as the original factory function
• ProjectionPlot becomes AxisAlignedProjectionPlot, making room for a "dispatch class" ProjectionPlot with the same dispatching pattern as SlicePlot

This eleviate the inconsistency, plus it allows users to rely on the fact that SlicePlot and ProjectionPlot are classes (as the camel case suggests).

I've checked that filenames generated by the PWViewerMPL.save method are consistent with the main branch.
The only possible backward compat breakage I can think of would be if anyone downstream relied on SlicePlot being a function, which seems terribly unlikely to me.

Note

The "dispatch class" pattern as implemented here is heavily inspired from the standard lib pathlib.Path class which instantiate pathlib.PosixPath or pathlib.WindowsPath depending on the OS, and provides a general abstraction for the two specialised classes.

PR Checklist

• New features are documented, with docstrings and narrative docs
• Adds a test for any bugs fixed. Adds tests for new features.

[neutrinoceros]

Same failures as seen in other recent PRs as well as on the daily run for Jenkins these days.

[neutrinoceros]

Close + reopen to retrigger tests

[neutrinoceros]

there may be a way to reduce the diff size of this PR using from __future__ import annotations.

[neutrinoceros]

I don't want to make this anymore confusing so I'll ignore my own previous comment.
The apparent diff is much bigger than the actual change, because I needed to move a couple classes and docstrings around to enable my proposed inheritance flow

[neutrinoceros]

Sigh. I realize my current reimplementation of the compatibility layer doesn't work:

class Base:
     def __new__(cls, *args, **kwargs):
         if 'hello' in kwargs:
             val = kwargs['hello']
             print(f"CATPURED VALUE {val}")
             del kwargs["hello"]
         self = object.__new__(cls)
         return self

     def __init__(self, *args, **kwargs):
         if 'hello' in kwargs:
             raise RuntimeError("this is not supposed to be possible")
Base(hello=True)

raises the "impossible" error. I need to reiterate on this

[neutrinoceros]

I think I solved this in the less disruptive fashion possible. Ideally I'd want to really align signatures for OffAxisSlicePlot.__init__ and AxisAlignedSlicePlot.__init__, but the way to do that is to make a handful of existing arguments keyword-only. While I think the sane way to use those is de facto by passing them as keywords, it is a disruption to existing users if anyone is using them as positional. I may propose such a change in the future but I don't want this PR to break backwards compatibility for anyone.

[neutrinoceros]

adding the bug label now that this fixes #3528

[matthewturk]

Seems pretty good. Anything in particular you want eyes on? I've read and it seems OK but there might be things I'm missing?

[neutrinoceros]

This is intended as a no-op for the most part, but I did change (supposedly improved) the way normal vectors are validated. I think this would be the part worth the most attention.

[matthewturk]

@chummels could you take a look, and if this looks OK to you can you merge?

[cphyc]

This looks great to me! Especially, I am very happy with the dispatching mechanism inserted in the __new__ method, very clean!

I however left few comments that I think should be addressed before this goes in.

[neutrinoceros]

@yt-fido test this please

[neutrinoceros]

@yt-fido test this please (this seems to keep hitting a randomly failing test in the athenapp frontend)

[neutrinoceros]

@yt-fido test this please (if this test fails one more time I'm making a PR to propose deactivating it because it's not just this PR)

edit: here's the PR #3620

[neutrinoceros]

So this has two approvals already but @chummels would like to review it too, so we'll hold this off for the time being :-)