Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
DOC: more nitpick follow up#15048
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The Axes classes have a multi-layered internal class structure andwe generate documentation for the top-level public classes. However,we also auto-generate property tables by querying the methods onthe objects which resolve to the base class they are defined on whichleads to incorrect references.This is a very quick hack which special-cases two of the Axes classesand replace their name with just ``Axes``.
@ImportanceOfBeingErnest RE#15042 (comment) can you try building the docs on windows from this branch? Sometimes the path includes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I built the docs off this branch locally, and it does not show the error I previously got.
| # sometimes the 'path' can contain ':' which are forbidden on | ||
| # windows, but on posix just passes through. | ||
| path, *post = path.partition(':') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
uh, how can there be colons in the path, even on posix? a priori this should be a real filesystem path given that we call Path(path).resolve() just below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, it's confusing that this is named "path", while in reality just being some string whichshould contain a path at the start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As always I'm a bit confused by the magic of escaping, but: appears to be kosher in posix paths:
sharon@21:09 ➤ touch "/tmp/path:o:log:i:cal"sharon@09:03 ➤ echo "hi there" > "/tmp/path:o:log:i:cal"sharon@09:03 ➤ ls /tmp/ | grep pathpath:o:log:i:calsharon@09:03 ➤ cat /tmp/path\:o\:log\:i\:cal hi thereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's certainly kosher (anything but / and \0 are for posix), but how is that relevant here? if we're running on linux, then the path is what it is and it will become relative to the mpl root just below (in relative_to), and there are no directories in mpl with colons; if running on windows, get_source_line should be returning a real path on the windows filesystem and that has no colons.
| @@ -1411,7 +1411,9 @@ def pprint_setters_rest(self, prop=None, leadingspace=4): | |||
| attrs = sorted(self._get_setters_and_targets()) | |||
| names = [self.aliased_name_rest(prop, target) | |||
| names = [self.aliased_name_rest(prop, target).replace( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it doesn't have to be this PR, but perhaps we should just fixAxes.__module__ to bematplotlib.axes, notmatplotlib.axes._base? seenumpy/numpy#12382 for similar stuff on numpy's side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That seems reasonable, but also push to another PR.
This fixes an exception just merged to master and the other half of
the issues@ImportanceOfBeingErnest is having in#14917.
PR Summary
PR Checklist