Skip to content

January 2023 NEWS Update.#1557

Merged
jkbonfield merged 5 commits intosamtools:developfrom
whitwham:news_jan_2023
Feb 16, 2023
Merged

January 2023 NEWS Update.#1557
jkbonfield merged 5 commits intosamtools:developfrom
whitwham:news_jan_2023

Conversation

@whitwham
Copy link
Member

Putting up the first draft now. Will put the final one in just before the release.

NEWS Outdated
Comment on lines 119 to 125
* Fix a bug in the codec learning algorithm for TOKA. The name tokeniser has a
rANS vs Arithmetic coder choice as a parameter (in the "strat" variable). We
lacked this distinction when learning which method works best, so in the
choice of toka (tok3+arith) vs bzip2 vs gzip etc we selected tok3 and switched
back to strat 0, disabling the arithmetic coder.his only affects archive mode,
or where requested eg "samtools view -O cram,version=3.1,use_arith".
(PR#1559)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think less is more with NEWS. If we're too detailed people simply won't bother to read it anyway, so it should be as brief as possible. The PR numbers let people jump to the full details if it's something they're interested in.

For intricacies like this I'd just paraphrase it in a single sentence. I'd suggest

"Fixed a bug in the codec parameter learning for CRAM 3.1 name tokeniser (PR#1559)"

NEWS Outdated
Comment on lines 115 to 116
* Catch errors from bgzf_getline in hts_readlist and hts_readlines. There were
a couple of places where the returned error was ignored.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly to below, we can cull the second sentence IMO.

NEWS Outdated
Comment on lines 93 to 95
* Fix error code returned by bcftools. Error code was returning 0 (success)
even on a fail. Replaced by -1.
(PR#1504, thanks to Lilian Janin)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is both too wordy and too content free. I understand where it comes from as it's the commit messages, but for PR#1504 all the actual explanation is in the conversation and not the poorly worded commit messages.

Maybe replace with:

"Bcftools no longer returns 0 exit status when encountering bgzf decode errors".

It may also be worth just bundling up a whole bunch of related things together. Eg

"Improved bcftools detection and reporting of bgzf decode errors"

That covers #1504, #1529, and #1554. Our readers don't really need to see the finer details of how the bugs were reported and fixed separately, given they're essentially all the same route cause of missing bgzf error handling.

NEWS Outdated
or where requested eg "samtools view -O cram,version=3.1,use_arith".
(PR#1559)

* CRAM improvements. Fixed crash with multi-threaded CRAM.Fixed a bug in the
Copy link
Contributor

@jkbonfield jkbonfield Feb 16, 2023

Choose a reason for hiding this comment

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

"multi-threaded CRAM.Fixed a bug" - missing space after full-stop. (Or two spaces, but that'll be my ancient brain.)

@jkbonfield
Copy link
Contributor

Looks gtg, apart from an additional space. :)

@jkbonfield jkbonfield merged commit 6652c86 into samtools:develop Feb 16, 2023
@whitwham whitwham deleted the news_jan_2023 branch September 29, 2023 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments