marcoabreu commented on issue #9771: Modify NDArrayIter constructor to receive 
tuple (i.e. dict in Python)?
URL: https://github.com/apache/incubator-mxnet/pull/9771#issuecomment-365223393
 
 
   Your change has the following issues, not providing backwards compatibility:
   ```
   Type change: data: IndexedSeq[NDArray] -> data: IndexedSeq[(String, NDArray)]
   Default value removed: private val dataBatchSize: Int = 1 -> private val 
dataBatchSize: Int
   Default value removed: shuffle: Boolean = false -> shuffle: Boolean
   Default value removed: lastBatchHandle: String = "pad" -> lastBatchHandle: 
String
   Argument removed: dataName: String = "data"
   Argument removed: labelName: String = "label"
   ```
   
   Backwards compatibility means that users using version 1.0 of MXNet, should 
not receive any errors when they upgrade to version 1.X. In your PR, you're 
modifying the constructor. This would throw an error for all users if they 
upgrade to the latest version. 
   
   Instead, you should add an additional overload of the constructor in order 
to support the old constructor as well as providing a new one using your 
proposed method. Speak, don't modify existing constructors but rather create 
another one.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to