split method parameter

[shawnwiggins]

Issue

The split method currently uses k as the parameter symbol. We noticed that this is causing some confusion since the method is primarily/only used during the $k$-NN section of the FDS.

Proposed Solution

Swap k with first_n (or something similar) in the source:

    def split(self, first_n):
        """Return a tuple of two tables where the first table contains
        ``first_n`` rows randomly sampled and the second contains the remaining rows.

        Args:
            ``first_n`` (int): The number of rows randomly sampled into the first
                table. ``first_n` must be between 1 and ``num_rows - 1``.

        Raises:
            ``ValueError``: ``first_n`` is not between 1 and ``num_rows - 1``.

        Returns:
            A tuple containing two instances of ``Table``.

        >>> jobs = Table().with_columns(
        ...     'job',  make_array('a', 'b', 'c', 'd'),
        ...     'wage', make_array(10, 20, 15, 8))
        >>> jobs
        job  | wage
        a    | 10
        b    | 20
        c    | 15
        d    | 8
        >>> sample, rest = jobs.split(3)
        >>> sample # doctest: +SKIP
        job  | wage
        c    | 15
        a    | 10
        b    | 20
        >>> rest # doctest: +SKIP
        job  | wage
        d    | 8
        """
        if not 1 <= first_n <= self.num_rows - 1:
            raise ValueError("Invalid value of first_n. first_n must be between 1 and the"
                             "number of rows - 1")

        rows = np.random.permutation(self.num_rows)

        first = self.take(rows[:first_n])
        rest = self.take(rows[first_n:])
        for column_label in self._formats:
            first._formats[column_label] = self._formats[column_label]
            rest._formats[column_label] = self._formats[column_label]
        return first, rest

[chitwankaur20]

To prevent confusion with k-NN, rather than renaming k to first_n, another approach could involve introducing a more descriptive alias (i.e., better convey what k is in the context of this specific method). num_rows_first_table: This also clearly communicates the purpose but is more verbose. This makes the code even more self-explanatory.

[v-hemish]

Hi @shawnwiggins. I’ve opened a PR to rename k→first_n: #636
PTAL! 🙏

[davidwagner]

Minor bikeshedding/pedagogical aspect: How about a name like train_size? or sample_size or num_rows? I don't find first_n very intuitive as a user/caller of split().

[shawnwiggins]

Minor bikeshedding/pedagogical aspect: How about a name like train_size? or sample_size or num_rows? I don't find first_n very intuitive as a user/caller of split().

@davidwagner: train_size sounds nice, and that is what sklearn uses for their train_test_split function:

sklearn.model_selection.train_test_split(*arrays, test_size=None, train_size=None, random_state=None, shuffle=True, stratify=None)

Hopefully, this will address the concern of @chitwankaur20, too. (I don't think any of the other datascience functions have agreement names as verbose as num_rows_first_table.)

[adnanhemani]

@shawnwiggins / @davidwagner I'm not sure if train_size makes as much sense in datascience pedagogically - while sklearn directly states their function as a way to generate a test/train split, datascience does not. I'd probably prefer num_rows though.

[geekyAman493]

I agree that the current parameter name k can be confusing, especially since it overlaps conceptually with k-NN usage elsewhere in the FDS.

After reviewing the proposed alternatives (first_n, sample_size, etc.), I’d recommend going with n_sample. It strikes a good balance between brevity and clarity, while staying consistent with conventions seen in NumPy and scikit-learn (e.g., n_neighbors, n_clusters, n_estimators).

Reasons for preferring n_sample:

Clearly communicates that it’s an integer count of rows to sample.

Consistent with other “n_” parameter patterns in data/ML codebases.

Keeps the function signature compact and readable (split(self, n_sample))