MAINT: Make output of Polynomial representations consistent#21760
MAINT: Make output of Polynomial representations consistent#21760rossbar merged 3 commits intonumpy:mainfrom
Conversation
|
@rossbar maybe you can have a look? @eendebakpt this change looks like it should get a basic tests. |
|
The necessary change is trivial: diff --git a/numpy/polynomial/_polybase.py b/numpy/polynomial/_polybase.py
index 920377535..483068cb5 100644
--- a/numpy/polynomial/_polybase.py
+++ b/numpy/polynomial/_polybase.py
@@ -490,7 +490,7 @@ def _repr_latex_(self):
if term_str == '1':
part = coef_str
else:
- part = rf"{coef_str}\,{term_str}"
+ part = rf"{coef_str}\cdot{term_str}"
if c == 0:
part = mute(part)I can take care of the tests if the change is approved. |
|
Also in ascii mode: >>> print(str(f))
23.98618858 + 23.21718647*(-1.0 + 0.22222222x) -
0.70392674*(-1.0 + 0.22222222x)**2 - 1.23554729*(-1.0 + 0.22222222x)**3diff --git a/numpy/polynomial/polynomial.py b/numpy/polynomial/polynomial.py
index 8e2c6f002..2cb96ee5d 100644
--- a/numpy/polynomial/polynomial.py
+++ b/numpy/polynomial/polynomial.py
@@ -1520,9 +1520,9 @@ def _str_term_unicode(cls, i, arg_str):
@staticmethod
def _str_term_ascii(i, arg_str):
if i == '1':
- return f" {arg_str}"
+ return f"*{arg_str}"
else:
- return f" {arg_str}**{i}"
+ return f"*{arg_str}**{i}"
@staticmethod
def _repr_latex_term(i, arg_str, needs_parens): |
|
Or maybe to be verbose about the '*' before 'x' as well >>> print(str(f))
23.98618858 + 23.21718647*(-1.0 + 0.22222222*x) -
0.70392674*(-1.0 + 0.22222222*x)**2 - 1.23554729*(-1.0 + 0.22222222*x)**3(in ascii mode only), so that after copy-paste it would be a valid python string that can be executed with no modifications. |
|
@axil Personally, I do not have a strong opinion on whether to use a dot, asterisk or no multiplication symbol (in either latex or str). But I am a bit hesitant to make changes to the current implementation. If there is consensus with another numpy dev I would be fine with making the changes. Or perhaps make quick poll on the numpy mailing list and decide which format to pick |
e817640 to
51fbaa8
Compare
51fbaa8 to
46c3457
Compare
46c3457 to
0548169
Compare
Basic test was added. @rossbar Could you have a look at this PR? |
0548169 to
0966675
Compare
0966675 to
cd6cb07
Compare
|
@seberg Do you want me to write release notes for this PR? I guess it should be with the |
0b18966 to
294e0a5
Compare
294e0a5 to
8f3a945
Compare
8f3a945 to
d1a561a
Compare
d1a561a to
bbb8733
Compare
bbb8733 to
fd5332e
Compare
6933d0a to
bb4a9ba
Compare
rossbar
left a comment
There was a problem hiding this comment.
Sorry for the delay @eendebakpt , this LGTM, pending the minor note about whether inline annotations are preferred. Will merge once that's cleared up!
| def _repr_latex_(self): | ||
| # get the scaled argument string to the basis functions | ||
| off, scale = self.mapparms() | ||
| def _format_term(self, scalar_format: Callable, off: float, scale: float): |
There was a problem hiding this comment.
AFAIK the policy is still to only include annotations in the pyi files - @seberg is this true?
There was a problem hiding this comment.
Dunno, have seen other code with private annotations which I think these are.
There was a problem hiding this comment.
If it doesn't matter then let's put it in, just wanted to make sure I wasn't missing any changes to the type annotation policies!
|
@eendebakpt seems this created a merge conflict (failing tests). Any chance you can look into that? |
|
I am seeing a new error after this was merged on some unrelated PR: I wonder how this passed, maybe it was not merged to main first? |
|
I will look into it |
|
This PRwas merged without merging with main first it seems. The latest time CI was executed was 5 months ago, so that might explains why we see some errors now. The |



This is a followup from #21653 and #21654. In this PR we make the output of
Polynomial.__str__andPolynomial._repr_latex_consistent. The representation forstrnow includes the domain-window mapping, which was hidden before.Input
Output before:
After:
Polynomial.fit()produces incorrect coefficients but plots correctly #24497 or numpy-discussion: Suggestions for changes to the polynomial module