[GitHub] [opennlp] rzo1 commented on a diff in pull request #469: OPENNLP-1423 Enhance JavaDoc in opennlp.tools.ml package

[GitBox <gi...@apache.org>]

rzo1 commented on code in PR #469:
URL: https://github.com/apache/opennlp/pull/469#discussion_r1056619744


##########
opennlp-tools/src/main/java/opennlp/tools/commons/Trainer.java:
##########
@@ -17,8 +17,36 @@
 
 package opennlp.tools.commons;
 
+import java.util.Map;
+
+import opennlp.tools.ml.AbstractTrainer;
+import opennlp.tools.ml.EventModelSequenceTrainer;
+import opennlp.tools.util.TrainingParameters;
+
 /**
  * Represents a common base for training implementations.
  */
 public interface Trainer {
+
+  /**
+   * Initializes a {@link AbstractTrainer} via {@link TrainingParameters} and
+   * a {@link Map report map}.
+   *
+   * @param trainParams The {@link Map} that maps training parameters on a key-value basis.
+   * @param reportMap The {@link Map} instance used as report map.
+   *                  
+   * @deprecated Use {@link #init(TrainingParameters, Map)} instead.
+   */
+  @Deprecated
+  void init(Map<String, Object> trainParams, Map<String, String> reportMap);

Review Comment:
   If it is `@Deprecated` (as it was pulled up from some implementing classes), why should we keep it in the interface, i.e. shouldn't we just remove it here and migrate the remaining four usages to the other signature?
   
   We already broke binary compatibility, so imho it would be a good opportunity to get rid of it instead of keeping it in the interface. Wdyt, @mawiesne @jzonthemtn ?



##########
opennlp-tools/src/main/java/opennlp/tools/ml/model/AbstractDataIndexer.java:
##########
@@ -188,17 +233,15 @@ protected List<ComparableEvent> index(ObjectStream<Event> events,
     return eventsToCompare;
   }
 
-  public int getNumEvents() {
-    return numEvents;
-  }
-
   /**
-   * Updates the set of predicated and counter with the specified event contexts and cutoff.
+   * Updates the set of predicates and counter with the specified event contexts and cutoff.
+   *
    * @param ec The contexts/features which occur in a event.
    * @param predicateSet The set of predicates which will be used for model building.
    * @param counter The predicate counters.
    * @param cutoff The cutoff which determines whether a predicate is included.
-   * @deprecated will be removed after 1.8.1 release
+   *
+   * @deprecated Use {{@link #update(String[], Map)}}. This method will be removed after 1.8.1 release

Review Comment:
   "1.8.1" did happen a while ago? maybe time to remove? wdyt?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org