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

DOC: update 1.5.0 release notes #12221

Merged
merged 4 commits into from
May 27, 2020
Merged

Conversation

tylerjereddy
Copy link
Contributor

@tylerjereddy tylerjereddy commented May 25, 2020

  • copy over the release notes from the wiki and reformat/clean up
    for rst format

  • use tools/authors.py to draft in the author list for release
    1.5.0

TODO:

  • fill in issue/PR lists when gh_lists.py is working: BUG, REL: gh_lists.py compromised scraping #12220
  • manually go through 1.5.0 PRs with ENH label and fill in missing entries (please do help)
  • adjust .mailmap for author name issues brought up in this PR [NO AUTHOR COMPLAINTS AT ALL]
  • finalize the "highlights" section of the release notes

@tylerjereddy tylerjereddy added the Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org label May 25, 2020
@tylerjereddy tylerjereddy added this to the 1.5.0 milestone May 25, 2020
@tylerjereddy tylerjereddy changed the title DOC: update 1.5.0 release notes WIP, DOC: update 1.5.0 release notes May 25, 2020
@tylerjereddy tylerjereddy added the needs-work Items that are pending response from the author label May 25, 2020
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Tyler. Looks like another solid release

doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
Generalized QR factorization routines (``?geqrf``) now have full ``_lwork``
counterparts.

`scipy.linalg.cossin` Cosine Sine decomposition of unitary matrices has been
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is pretty bad, I'd expect something named cossin to do an elementwise math operation rather than a decomposition. Should we rename it before the release?

Copy link
Member

@ilayn ilayn May 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we had this discussion but we couldn't came up with anything better and it is kind of what everybody goes with elsewhere. So we counted on the fact that it is under linalg namespace. But yes I would really appreciate a better one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either csd (in analogy with svd) or a more explicit cossin_decomp would be better imho

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

csd is already overloaded in scipy.signal.csd and some circles already use it for a shortcut to cumulative power spectral density. So that won't fly

Frankly, cossin_decomp is even uglier than cossin also becomes a one-off since the others don't have it.

Why I prefer cossin is that there is nothing called cossin in any trig libraries. And you can't accidentally fall to that namespace.

doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
``w, v, info`` to ``w, v, m, isuppz, info``

The order of output arguments ``w``, ``v`` of ``<sy/he>{gv, gvd, gvx}`` is
swapped.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting here for further review: check if these give hard errors; sounds like silently changing numerical values now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the argument order is changed to match the remaining ones. So I don't know if we can raise errors based on that or how.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do have different shape at least:

w : rank-1 array('d') with bounds (n)
v : rank-2 array('d') with bounds (n,n) and a storage

Still, it sounds like something to avoid. Was there a reason why it had to be done (e.g. infrastructure for a whole family of functions), or was it simply "let's make it consistent"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably they were hardly ever used. Because their lwork argument was not correct. So both fixed and made consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay that makes sense - I can try to add a sentence so that that bit doesn't read like people should go audit their code because we did something unusual.

doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
Copy link
Member

@AtsushiSakai AtsushiSakai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the PRs that I created. If there is anything I can do, let me know.

doc/release/1.5.0-notes.rst Show resolved Hide resolved
doc/release/1.5.0-notes.rst Show resolved Hide resolved
doc/release/1.5.0-notes.rst Show resolved Hide resolved
doc/release/1.5.0-notes.rst Show resolved Hide resolved
@tylerjereddy tylerjereddy force-pushed the relnotes_150 branch 2 times, most recently from bf547f4 to 1cf8d83 Compare May 26, 2020 16:23
@tylerjereddy
Copy link
Contributor Author

@AtsushiSakai Ok, I condensed the large number of input validation/exception handling improvements to a single broad statement.

@tylerjereddy
Copy link
Contributor Author

Ok, I did the "manual sweep" of 1.5.0 PRs with enhancement labels and added a dozen or so additional things that were missed.

Also, tried to fix the doc build errors because of rst changes here.

doc/release/1.5.0-notes.rst Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
doc/release/1.5.0-notes.rst Outdated Show resolved Hide resolved
tylerjereddy and others added 4 commits May 26, 2020 20:32
* copy over the release notes from the wiki and reformat/clean up
for `rst` format

* use `tools/authors.py` to draft in the author list for release
`1.5.0`
Co-authored-by: Warren Weckesser <warren.weckesser@gmail.com>
Co-authored-by: Josh Wilson <person142@users.noreply.github.com>
* add some missing release note ENH PR entries

* address some reviewer comments

* some fixes for doc build errors with `make html-scipyorg`
due to the release notes update

* add issue/PR lists
@tylerjereddy tylerjereddy changed the title WIP, DOC: update 1.5.0 release notes DOC: update 1.5.0 release notes May 27, 2020
@tylerjereddy tylerjereddy removed the needs-work Items that are pending response from the author label May 27, 2020
@tylerjereddy
Copy link
Contributor Author

If the CI is green after most recent update I will most likely merge.

@tylerjereddy
Copy link
Contributor Author

We can consider backporting for unresolved matters if we really have to.

The ``binned_statistic_dd`` function with ``statistic="std"`` performance was
improved by ~4x.

``scipy.stats.kstest(rvs, cdf,...)`` now handles both one-sample and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps put the 'kstest' higher up next to the 'kstwo' item as they both relate to K-S statistics?

Comment on lines +219 to +221
The ``axis`` parameter was added to `scipy.stats.f_oneway`, allowing it compute
multiple one-way ANOVA tests for data stored in n-dimensional arrays. Some
performance improvements to ``f_oneway`` as well.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the missing word to, and make the last phrase a sentence.

Suggested change
The ``axis`` parameter was added to `scipy.stats.f_oneway`, allowing it compute
multiple one-way ANOVA tests for data stored in n-dimensional arrays. Some
performance improvements to ``f_oneway`` as well.
The ``axis`` parameter was added to `scipy.stats.f_oneway`, allowing it to
compute multiple one-way ANOVA tests for data stored in n-dimensional
arrays. The performance of ``f_oneway`` was also improved for some cases.

@tylerjereddy tylerjereddy merged commit b1e1872 into scipy:master May 27, 2020
@tylerjereddy tylerjereddy deleted the relnotes_150 branch May 27, 2020 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants