SymPy 1.9 release

[Oscar Benjamin]

Hi all,

I want to put out the SymPy 1.9 release ASAP.

There have been a large number of changes since 1.8:
https://github.com/sympy/sympy/wiki/Release-Notes-for-1.9

I intend to spend a little time editing those notes before the release
but I don't have time to go through all of them. Contributors please
check the release notes for the changes you have made and improve them
if needed (anyone can edit the wiki). In some cases it might make
sense to merge a few related release notes if they come from multiple
related PRs.

The issues here are the release blockers:
https://github.com/sympy/sympy/milestone/59

Anyone is welcome to have an opinion on the printing of Heaviside functions:
https://github.com/sympy/sympy/issues/21945
In the absence of any other opinions I'll just close the issue though.

Just now I discovered a regression:
https://github.com/sympy/sympy/issues/22058
That will need to be fixed.

If anyone knows of anything else that should be fixed before the
release please say so. Otherwise as soon as 22058 is fixed I'll close
21945 and put out a release candidate.

Also GSOC students: Can you please each add a paragraph to the
"highlights" section of the release notes explaining what was achieved
overall from the projects? I will edit what you write so it doesn't
need to be perfect but I can't just write that for you because for
most projects I don't know what you actually did and even for the ones
I supervised it's hard to remember all the things that were done.
https://github.com/sympy/sympy/wiki/Release-Notes-for-1.9#highlights

If anyone else knows of particularly significant changes that should
be highlighted then please let me know.


Oscar

[Aaron Meurer]

Thank you for working on this as always Oscar.

On Thu, Sep 9, 2021 at 2:25 PM Oscar Benjamin
<oscar.j....@gmail.com> wrote:
>
> Hi all,
>
> I want to put out the SymPy 1.9 release ASAP.
>
> There have been a large number of changes since 1.8:
> https://github.com/sympy/sympy/wiki/Release-Notes-for-1.9
>
> I intend to spend a little time editing those notes before the release
> but I don't have time to go through all of them. Contributors please
> check the release notes for the changes you have made and improve them
> if needed (anyone can edit the wiki). In some cases it might make
> sense to merge a few related release notes if they come from multiple
> related PRs.
>
> The issues here are the release blockers:
> https://github.com/sympy/sympy/milestone/59
>
> Anyone is welcome to have an opinion on the printing of Heaviside functions:
> https://github.com/sympy/sympy/issues/21945
> In the absence of any other opinions I'll just close the issue though.

I also commented this on the issue, but why was the default value for
Heaviside(0) changed (previously it was unevaluated, but now
Heaviside(0) is 1/2)? As I explained in the issue, I think this could
negatively impact people's existing code, so we should only make the
change if there is a good reason.

More generally, for release notes entries for backwards compatible
changes, I think we should explain not just what changed but why it
was changed, and why it was considered important enough to break
compatibility. None of the entries listed in the 1.9 backwards
compatible breaks section really do that, except for the one about
sample() returning to pre-1.7 behavior.

Aaron Meurer

>
> Just now I discovered a regression:
> https://github.com/sympy/sympy/issues/22058
> That will need to be fixed.
>
> If anyone knows of anything else that should be fixed before the
> release please say so. Otherwise as soon as 22058 is fixed I'll close
> 21945 and put out a release candidate.
>
> Also GSOC students: Can you please each add a paragraph to the
> "highlights" section of the release notes explaining what was achieved
> overall from the projects? I will edit what you write so it doesn't
> need to be perfect but I can't just write that for you because for
> most projects I don't know what you actually did and even for the ones
> I supervised it's hard to remember all the things that were done.
> https://github.com/sympy/sympy/wiki/Release-Notes-for-1.9#highlights
>
> If anyone else knows of particularly significant changes that should
> be highlighted then please let me know.
>
>
> Oscar
>

> --
> You received this message because you are subscribed to the Google Groups "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sympy+un...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAHVvXxRU0VKJhxZc%2B8q643txB7XS5zjseeQxtqopUhK0J-t8wQ%40mail.gmail.com.

[Oscar Benjamin]

"
On Thu, 9 Sept 2021 at 23:12, Aaron Meurer <asme...@gmail.com> wrote:
>
> On Thu, Sep 9, 2021 at 2:25 PM Oscar Benjamin
> <oscar.j....@gmail.com> wrote:
> >
> > Anyone is welcome to have an opinion on the printing of Heaviside functions:
> > https://github.com/sympy/sympy/issues/21945
> > In the absence of any other opinions I'll just close the issue though.
>
> I also commented this on the issue, but why was the default value for
> Heaviside(0) changed (previously it was unevaluated, but now
> Heaviside(0) is 1/2)? As I explained in the issue, I think this could
> negatively impact people's existing code, so we should only make the
> change if there is a good reason.

There are lots of issues/PRs where this was discussed but probably the
final discussion was here:
https://github.com/sympy/sympy/pull/20411

And the PR that made the change was here:
https://github.com/sympy/sympy/pull/21452

A significant part of the motivation was as a prerequisite for this PR
making it possible to lambdify Heaviside which would fail for H(0)
unless a decision was made about what value it should have:
https://github.com/sympy/sympy/pull/21573

The basic reason why it's a good idea is just because it means that
the standard 1-arg form of the function is well-defined.

> More generally, for release notes entries for backwards compatible
> changes, I think we should explain not just what changed but why it
> was changed, and why it was considered important enough to break
> compatibility. None of the entries listed in the 1.9 backwards
> compatible breaks section really do that, except for the one about
> sample() returning to pre-1.7 behavior.

Yes, they should be improved. That's the part of the release notes
that I intend to focus on.

Some of the release notes listed under the backwards compatibility
section are actually not incompatible changes at all e.g. "Moment no
more has None in args" is just a bug fix that doesn't need to be
listed. I don't even think that particular example needs an ordinary
release note.

Others like the changes to rewrite and Predicate are unlikely to be
noticed by anyone. They are listed as compatibility breaks when they
are really just internal changes. Unfortunately the SymPy
documentation does not make it clear what is internal and what is not
so it is hard to judge whether any particular change needs to be
listed as a compatibility break or not.

You can see another example in the release notes where I wrote:

- The (internal) PolyMatrix class has been changed and now requires
generators to be provided on construction. (#21441 by @oscarbenjamin)

Here I need to clarify that the class is internal. I have definitely
seen people trying to use this class though so even though it is
internal someone is using it for something. There have been changes to
it that are not at all compatible. The SymPy docs don't make clear
what's internal or not though and regardless people will just go into
the code and use whatever they find if they think it is the right
thing:
https://stackoverflow.com/a/58124273/9450991

I decided that the changes made to PolyMatrix did not need to be
listed as a compatibility break because no one should have been using
that class. I know however that some people are using it though so I
at least added a release note explaining that there was a change to an
internal class. I think some of the other "compatibility breaks" could
also be listed in the same way.

The question is how cautious do you want to be about describing
something as an incompatible change. In a formal sense no change is
entirely compatible so there needs to be some judgement about what is
worth mentioning or what is worth adding a DeprecationWarning for
(rather than simply making the change).

--
Oscar

[Aaron Meurer]

On Thu, Sep 9, 2021 at 6:07 PM Oscar Benjamin

<oscar.j....@gmail.com> wrote:
>
> "
> On Thu, 9 Sept 2021 at 23:12, Aaron Meurer <asme...@gmail.com> wrote:
> >
> > On Thu, Sep 9, 2021 at 2:25 PM Oscar Benjamin
> > <oscar.j....@gmail.com> wrote:
> > >
> > > Anyone is welcome to have an opinion on the printing of Heaviside functions:
> > > https://github.com/sympy/sympy/issues/21945
> > > In the absence of any other opinions I'll just close the issue though.
> >
> > I also commented this on the issue, but why was the default value for
> > Heaviside(0) changed (previously it was unevaluated, but now
> > Heaviside(0) is 1/2)? As I explained in the issue, I think this could
> > negatively impact people's existing code, so we should only make the
> > change if there is a good reason.
>
> There are lots of issues/PRs where this was discussed but probably the
> final discussion was here:
> https://github.com/sympy/sympy/pull/20411

I guess I stopped following this PR around when this change was made.
I would have been opposed to it then if I had seen it.

Making Heaviside default to 1/2 just for lambdify would be an OK
change. I suggested several alternate ways of specifying the value of
Heaviside(0) in that PR. But none of them would be backwards
incompatible because lambdify() didn't support Heaviside at all before
that PR.

>
> And the PR that made the change was here:
> https://github.com/sympy/sympy/pull/21452
>
> A significant part of the motivation was as a prerequisite for this PR
> making it possible to lambdify Heaviside which would fail for H(0)
> unless a decision was made about what value it should have:
> https://github.com/sympy/sympy/pull/21573
>
> The basic reason why it's a good idea is just because it means that
> the standard 1-arg form of the function is well-defined.

The previous behavior of Heaviside(0) being unevaluated is also
well-defined. The only way that becomes problematic is if you need a
numeric value for the expression, then neither evalf() nor lambdify()
will produce a value. But I personally always saw this as a feature.
You must explicitly specify your desired value for Heaviside(0).

My biggest concern here is that this breaks compatibility, and I'm not
sure it's necessary to do so. If we had started with the behavior we
now have in master, that might have been fine, although I personally
think the old behavior with Heaviside(0) unevaluated was better.

To be fair, I think there is some expectation that if you dig up some
random undocumented class from a codebase that it is probably not
really public API. People still use private APIs even when they know
they are private if it's the only thing that can solve their problem.
I'm not saying we shouldn't be clear about this, especially given the
discussions around backwards compatibility breaks, but the main signal
I would take from this is that something like this would be useful to
have in the public API, even if it isn't this specific class.

Aaron Meurer

>
> I decided that the changes made to PolyMatrix did not need to be
> listed as a compatibility break because no one should have been using
> that class. I know however that some people are using it though so I
> at least added a release note explaining that there was a change to an
> internal class. I think some of the other "compatibility breaks" could
> also be listed in the same way.
>
> The question is how cautious do you want to be about describing
> something as an incompatible change. In a formal sense no change is
> entirely compatible so there needs to be some judgement about what is
> worth mentioning or what is worth adding a DeprecationWarning for
> (rather than simply making the change).
>
> --
> Oscar
>

> --
> You received this message because you are subscribed to the Google Groups "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sympy+un...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAHVvXxS%3DW2QzD-HsePuR9B6QARUpUoTcKq7tdqcPuUU8rPto4Q%40mail.gmail.com.

[Oscar Benjamin]

On Fri, 10 Sept 2021 at 02:02, Aaron Meurer <asme...@gmail.com> wrote:
>
> On Thu, Sep 9, 2021 at 6:07 PM Oscar Benjamin
> <oscar.j....@gmail.com> wrote:
> >
> > "
> > On Thu, 9 Sept 2021 at 23:12, Aaron Meurer <asme...@gmail.com> wrote:
> > >
> > > On Thu, Sep 9, 2021 at 2:25 PM Oscar Benjamin
> > > <oscar.j....@gmail.com> wrote:
> > > >
> > > > Anyone is welcome to have an opinion on the printing of Heaviside functions:
> > > > https://github.com/sympy/sympy/issues/21945
> > > > In the absence of any other opinions I'll just close the issue though.
> > >
> > > I also commented this on the issue, but why was the default value for
> > > Heaviside(0) changed (previously it was unevaluated, but now
> > > Heaviside(0) is 1/2)? As I explained in the issue, I think this could
> > > negatively impact people's existing code, so we should only make the
> > > change if there is a good reason.
> >
> > There are lots of issues/PRs where this was discussed but probably the
> > final discussion was here:
> > https://github.com/sympy/sympy/pull/20411
>
> I guess I stopped following this PR around when this change was made.
> I would have been opposed to it then if I had seen it.
>
> Making Heaviside default to 1/2 just for lambdify would be an OK
> change. I suggested several alternate ways of specifying the value of
> Heaviside(0) in that PR. But none of them would be backwards
> incompatible because lambdify() didn't support Heaviside at all before
> that PR.

I don't think that having a lambdify give different values from the
sympy expression is a good choice. I don't know if this is documented
anywhere but my basic expectation ot lambdify is that it should give
the same values as sympy apart from unavoidable differences due to
rounding errors. Having H(0) undefined but with lambdify arbitrarily
choosing different values seems like a basic breach of contract to me.

> > And the PR that made the change was here:
> > https://github.com/sympy/sympy/pull/21452
> >
> > A significant part of the motivation was as a prerequisite for this PR
> > making it possible to lambdify Heaviside which would fail for H(0)
> > unless a decision was made about what value it should have:
> > https://github.com/sympy/sympy/pull/21573
> >
> > The basic reason why it's a good idea is just because it means that
> > the standard 1-arg form of the function is well-defined.
>
> The previous behavior of Heaviside(0) being unevaluated is also
> well-defined. The only way that becomes problematic is if you need a
> numeric value for the expression, then neither evalf() nor lambdify()
> will produce a value. But I personally always saw this as a feature.
> You must explicitly specify your desired value for Heaviside(0).
>
> My biggest concern here is that this breaks compatibility, and I'm not
> sure it's necessary to do so. If we had started with the behavior we
> now have in master, that might have been fine, although I personally
> think the old behavior with Heaviside(0) unevaluated was better.

I don't think that it does really break compatibility in a significant
way. I haven't seen any user say that they appreciate the behaviour of
H(0) and I think that for most users who notice H(0) is just annoying.
A lot of them don't care what value H(0) takes but they don't want it
to blow up on them. Those that do care have a clear option: use the
second argument to specify the value like H(x, 0) or H(x, nan) or
whatever you want. That was the way to handle it before and it's still
the way to handle it now.

--
Oscar

[Oscar Benjamin]

On Fri, 10 Sept 2021 at 02:02, Aaron Meurer <asme...@gmail.com> wrote:
>
> On Thu, Sep 9, 2021 at 6:07 PM Oscar Benjamin
> <oscar.j....@gmail.com> wrote:
> >
> > You can see another example in the release notes where I wrote:
> >
> > - The (internal) PolyMatrix class has been changed and now requires
> > generators to be provided on construction. (#21441 by @oscarbenjamin)
> >
> > Here I need to clarify that the class is internal. I have definitely
> > seen people trying to use this class though so even though it is
> > internal someone is using it for something. There have been changes to
> > it that are not at all compatible. The SymPy docs don't make clear
> > what's internal or not though and regardless people will just go into
> > the code and use whatever they find if they think it is the right
> > thing:
> > https://stackoverflow.com/a/58124273/9450991
>
> To be fair, I think there is some expectation that if you dig up some
> random undocumented class from a codebase that it is probably not
> really public API. People still use private APIs even when they know
> they are private if it's the only thing that can solve their problem.
> I'm not saying we shouldn't be clear about this, especially given the
> discussions around backwards compatibility breaks, but the main signal
> I would take from this is that something like this would be useful to
> have in the public API, even if it isn't this specific class.

Well PolyMatrix is now just a wrapper around DomainMatrix which is
documented and has well-defined behaviour. So DomainMatrix can be used
directly although it is less convenient.

Someone could take PolyMatrix and make it work better but I think that
actually what is needed is a Matrix class that can be used in place of
both PolyMatrix and normal Matrix. Basically a wrapper around
DomainMatrix where you can specify the domain but it has the
convenient usability of the other Matrix classes (e.g. element access
gives Expr).

So far I've been focussing on uniting/removing matrix classes rather
than adding new ones though. As of 1.9 the RawMatrix and NewMatrix
classes are removed (DomainMatrix used instead) and PolyMatrix,
Matrix, ImmutableMatrix, SparseMatrix and ImmutableSparseMatrix are
all just wrappers around the DomainMatrix class. None of those changes
affects public API but introducing a new public matrix class is a
significant API change that should be considered carefully because it
could lead to a lot of user confusion.

Actually I just realised that RawMatrix is not actually removed. It's
not used anywhere any more but it's still there although it will give
a DeprecationWarning. That's another example of overthinking these
things: adding a DeprecationWarning for a broken internal undocumented
class that barely works and that no one should be using.

--
Oscar