search

nimble-dev / nimble

The base NIMBLE package for R
http://R-nimble.org
BSD 3-Clause "New" or "Revised" License
156 stars 23 forks source link

Enhance compile-time and run-time error-trapping of size mismatches for assignment operator #1459

Closed perrydv closed 2 months ago

perrydv commented 3 months ago

This aims to fix #1425 .

It puts a bunch of new logic in sizeAssignAfterRecursing to catch compile-time and run-time mismatches when a vector or matrix with indexing on the LHS is assigned from another vector or matrix. All the new logic is behind a new nimbleOption so it can be toggled off if it ever causes problems. There is a very real possibility that this will break something in testing as assignment processing is pervasive.

paciorek commented 3 months ago

We think this is basically good to go (test failures look unrelated) but decided to leave out of 1.2.0 in last push to release.

paciorek commented 2 months ago

@perrydv well we don't have much time before 1.2.1 release, but perhaps this is the time to do this? Given it's a minor release we won't be strongly encouraging folks to upgrade.

@perrydv @danielturek any thoughts?

danielturek commented 2 months ago

@paciorek Given that (a) it's a minor release, (b) this is protected behind an option, and (c) this is a useful improvement, I would vote to include it.

Furthermore, it's easier now to include it for 1.2.1, then if problems arise, fix (or remove) it for 1.3.0. But it's more annoying if it causes problems in an initial 1.3.0 release.

That said, I defer to either of you.

paciorek commented 2 months ago

Yes, that was my thinking regarding doing now vs. 1.3.0. I'll proceed with this.

perrydv commented 2 months ago

I'm ok with the decision here.