search

bedatadriven / renjin

JVM-based interpreter for the R language for the statistical analysis.
https://www.renjin.org
GNU General Public License v2.0
513 stars 82 forks source link

RowNamesVector construction #544

Closed perNyfelt closed 2 years ago

perNyfelt commented 2 years ago

In v0.9.2726 (and beta-76) the RowNamesVector had several more constructors then what is in master. The RJDBC package for instance uses the RowNamesVector(int numRows) constructor which is no longer available in master. The odd thing is that the RowNamesVector in master looks just like it used to do a long time ago, before 792d81735d1f80d48eee0964322715ebc4c7f502 commited on 1/5/20 by Alex. It is almost that some old code snuck back in - or maybe it was intentional.

The RowNamesVector currently looks like this: https://github.com/bedatadriven/renjin/blob/master/core/src/main/java/org/renjin/primitives/vector/RowNamesVector.java

But the v0.9.2726 version looks like this: https://github.com/bedatadriven/renjin/blob/v0.9.2726/core/src/main/java/org/renjin/primitives/vector/RowNamesVector.java

Could anyone in the know comment on whether this is intentional or a mistake?

perNyfelt commented 2 years ago

I change the JDBCUtils in RJDBC to do

  1. Create a list of String and fill it with the row number for each row
  2. Changed from a RowNamesVector when adding "row.names" attribute in the ListVector to a StringArrayVector i.e:
StringVector vec = new StringArrayVector(rowNames);
dataFrame.setAttribute("row.names", vec);

...which seems to work fine.

perNyfelt commented 2 years ago

I had an older version of renjin-dbi and see that this is already resolved in a similar way (with new ConvertingStringVector(IntSequence.fromTo(1, rows))). So this must be the intended behavior.