[ENH] V1 → V2 API Migration - datasets by JATAYU000 · Pull Request #1608 · openml/openml-python

JATAYU000 · 2026-01-08T10:30:37Z

Metadata

Reference Issue: [ENH] V1 → V2 API Migration - datasets #1592
Depends on: [ENH] V1 → V2 API Migration - core structure #1576
Change Log Entry:This PR implements Datasets resource, and refactor its existing functions

codecov-commenter · 2026-01-08T10:36:04Z

Codecov Report

❌ Patch coverage is 83.45382% with 206 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.23%. Comparing base (8a5532f) to head (77d2af2).

Files with missing lines	Patch %	Lines
openml/_api/resources/dataset.py	80.12%	95 Missing ⚠️
openml/_api/clients/http.py	84.50%	33 Missing ⚠️
openml/_api/clients/minio.py	60.00%	28 Missing ⚠️
openml/_api/setup/backend.py	80.70%	11 Missing ⚠️
openml/_api/resources/base/versions.py	87.01%	10 Missing ⚠️
openml/_config.py	87.09%	8 Missing ⚠️
openml/_api/resources/base/fallback.py	84.21%	6 Missing ⚠️
openml/_api/setup/builder.py	86.66%	6 Missing ⚠️
openml/cli.py	0.00%	4 Missing ⚠️
openml/datasets/dataset.py	91.66%	2 Missing ⚠️
... and 3 more

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1608       +/-   ##
===========================================
+ Coverage   52.73%   80.23%   +27.49%     
===========================================
  Files          37       61       +24     
  Lines        4399     5369      +970     
===========================================
+ Hits         2320     4308     +1988     
+ Misses       2079     1061     -1018

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JATAYU000 · 2026-01-09T05:08:45Z

FYI @geetu040 Currently the get_dataset() function has 3 download requirement

download_data : uses api_calls._download_minio_bucket() to download all the files in the bucket if download_all_files param was True and api_calls._download_minio_file() to download the dataset.pq file if it was not found in cache. When download parquet fails it fallback to download dataset.arff file with get request
download_features : if feature_file is passed via init it extracts during initialization else does get request and caches the xml
download_qualities : if qulities_file is passed via init it extracts during initialization else does get request and caches the xml

Issues:

The data files .pq and .arff are common for versions and doesn't make sense to be downloaded multiple times
Path handling for download to return the path especially the data files, As mentioned in the meet I can try the Download specific class which uses the cache mixin and only inherited by dataset resource.
Current implementation in OpenMLDataset has v1 specific parsing which in my opinion should be using the current interface (api_context)

Example:

current load_features() ref link
This calls a function which downloads and returns a file path and then parse from the file path
This can be changed by changing that function's definition ref link to get -> parse -> return features instead of file paths

def _get_dataset_features_file(did_cache_dir: str | Path | None, dataset_id: int) -> dict[int, OpenMLDataFeature]:
        return _features

Or by updating the Dataset class to use the underlining interface method from api_context directly.

def _load_features(self) -> None:
       ...
        self._features = api_context.backend.datasets.get_features(self.dataset_id)

Another option is to add return_path to client requests, which in my opinion would be wasteful since adding a param to all the methods of client for just the dataset resource, and that too which could be handled without it as mentioned above.

geetu040

Left an intermediate review. This is solid work and well done overall. Nice job. I'll look into the download part now.

geetu040 · 2026-01-13T17:43:32Z

openml/_api/resources/base/resources.py

+    def list(
+        self,
+        limit: int,
+        offset: int,
+        *,
+        data_id: list[int] | None = None,  # type: ignore
+        **kwargs: Any,
+    ) -> pd.DataFrame: ...


can we not have same signature for all 3 methods: DatasetsAPI.list, DatasetsV1.list, DatasetsV2.list? does it raise pre-commit failures since a few might not be used?

Oh that v2 signature was experimental, idk how pre-commits did not catch that, Will make them same

Is mypy supposed to catch that?

yes unused parameters are caught under #ARG001 as seen in the cache_directory params.

geetu040 · 2026-01-13T17:43:43Z

openml/_api/resources/dataset.py

+    def list(
+        self,
+        limit: int,
+        offset: int,
+        *,
+        data_id: list[int] | None = None,  # type: ignore
+        **kwargs: Any,
+    ) -> pd.DataFrame:


you can make this simple using private helper methods

@geetu040 I could not find a good common block to make an helper since the filters are passed via url in v1 and via json object in v2 , and both have different parsing, If you have any specific Idea on that please let me know

looking at DatasetsV1 I can think of these helper methods: _build_list_call, _parse_and_validate_xml, _parse_dataset
you can do something similar for DatasetsV2 though they can be different.

you can do something similar for DatasetsV2 though they can be different.

I see, That opens more options.

geetu040 · 2026-01-13T17:43:53Z

openml/_api/resources/datasets.py

+        bool
+            True if the deletion was successful. False otherwise.
+        """
+        return openml.utils._delete_entity("data", dataset_id)


if you implement the delete logic yourself instead of openml.utils._delete_entity, how would that look? I think it would be better.

Makes Sense , It would look like a delete request from client along with exception handling

geetu040 · 2026-01-13T17:43:57Z

openml/_api/resources/datasets.py

+    def list(
+        self,
+        limit: int,
+        offset: int,
+        **kwargs: Any,
+    ) -> pd.DataFrame:


same as above, it can use private helper methods

geetu040 · 2026-01-13T17:44:00Z

openml/datasets/functions.py

-    # Minimalistic check if the XML is useful
-    if "oml:data_qualities_list" not in qualities:
-        raise ValueError('Error in return XML, does not contain "oml:data_qualities_list"')
+    from openml._api import api_context


can't we have this import at the very top? does it create circular import error? if not, should be moved to top from all functions.

It does raise circular import

geetu040 · 2026-01-14T09:16:26Z

FYI @geetu040 Currently the get_dataset() function has 3 download requirement

Thanks for a detailed explanation, I now have good understanding of the download mechanism.

download_data : uses api_calls._download_minio_bucket() to download all the files in the bucket if download_all_files param was True and api_calls._download_minio_file() to download the dataset.pq file if it was not found in cache. When download parquet fails it fallback to download dataset.arff file with get request

minio can be handled easily, we will use a separate client along with HTTPClient or implement it's methods in the HTTPClient, which work independently of the api version

download_features : if feature_file is passed via init it extracts during initialization else does get request and caches the xml

download_qualities : if qulities_file is passed via init it extracts during initialization else does get request and caches the xml

these are actually different objects in both apis, v1 uses xml and v2 keeps them in json

The data files .pq and .arff are common for versions and doesn't make sense to be downloaded multiple times

yes you are right, they are the same files, which are not required to be downloaded again for both versions, but isn't this true for almost all the http objects? they may have different format xml or json, slightly different structure, but if parsed most are identical, so shouldn't this rule be applied to all the responses?

Path handling for download to return the path especially the data files, As mentioned in the meet I can try the Download specific class which uses the cache mixin and only inherited by dataset resource.

I don't understand this point

Current implementation in OpenMLDataset has v1 specific parsing which in my opinion should be using the current interface (api_context)

agreed, should be handled by api_context

Another option is to add return_path to client requests, which in my opinion would be wasteful since adding a param to all the methods of client for just the dataset resource, and that too which could be handled without it as mentioned above.

agreed, adding return_path for just one specific method of one resource is not preffered

in conclusion, I may ask, if we ignore the fact that it downloads the .arff files for both versions separately, does everything else works out smooth without any blocker? I think ignoring this part is not really bad because conceptually this rule could be applied to almost every other response object

JATAYU000 · 2026-01-15T05:20:18Z

@geetu040 making a new client for minio handles just the parquet file, we would still need to migrate download_text_file() for the arff file (this is also used by tasks and runs)
So maybe we can have a DownloadClient which can contain all of these along with a save method which can save content to a specified path and hence also fixes our issue with features/qualities path ?

FYI the new commit adds better handling of feature and qualites in OpenMLDataset class moving the v1 specific parsing logic to the interface. So only part left is to handle

return path of saved file (feature, qualities, arff, pq)
downloader for arff or implementation of download_text_file() which is used for arff download
minio file and bucket download for the pq file

geetu040 · 2026-01-15T16:24:02Z

From the standup discussion and earlier conversations, I think we can agree on a few points:

Have a separate client for MinIO interactions alongside HTTPClient. In future if we plan to add more providers like dropbox, google drive, e.t.c, we don't end up with too many changes, rather have their client implemented as a separate class and just pass that down to relevant resource.
DownloadClient doesn't feel like the right abstraction; instead, implement download-specific methods directly in HTTPClient.

Consider this a green light to experiment with the client design. Try an approach, use whatever caching strategy you think fits best, and aim for a clean, sensible design. Feel free to ask for suggestions or reviews along the way. I'll review it in code. Just make sure this doesn't significantly impact the base classes or other stacked PRs.

JATAYU000 · 2026-01-16T04:35:27Z

The points do make sense to me, I will propose the design along with how it would be used in the resource.

JATAYU000 · 2026-01-19T06:08:02Z

@geetu040 I have a design implemented which needs reviews

MinIOClient similar to HTTPClient is being used by DatasetAPI from self._minio , It implements 2 methods download file and download bucket, it uses _get_cache_dir() for the destination
New method download implemented under HTTPclient that can be used for features,qualities and arff files, along with specific v1/v2 interface using handler callback.

Question:

most methods signature include cache_directory how should that be handled? if the directory is passed use that if not use our cache dir? i am not sure how this would effect the old users
Also the caching implemented currently suggest the Response() is cached but I remember from a meeting you mentioned the respective files (.xml .json) are cached, I am not sure about it , I have went through the design as if the caching is done on the response.

geetu040 · 2026-01-19T07:40:19Z

@geetu040 I have a design implemented which needs reviews

I have taken a quick look, the design looks really good, though I have some suggestions/questions in the code, which I will leave in a detailed review. But this in general fixes all our blockers without damaging the original design.

most methods signature include cache_directory how should that be handled? if the directory is passed use that if not use our cache dir? i am not sure how this would effect the old users

Is it provided by the user? I don't think so. In that case, how does it affect the users? From looking at the code, this cache directory is generated programmatically inside the functions, we can completely remove these lines and always rely on the CacheMixin class. How does that sound?

Also the caching implemented currently suggest the Response() is cached but I remember from a meeting you mentioned the respective files (.xml .json) are cached, I am not sure about it , I have went through the design as if the caching is done on the response.

CacheMixin._set_cache_response will look at the response object and extract json or xml content from it and save it respectively in .json and .xml files.
CacheMixin._get_cache_response will read these files (.json or .xml) from the given path and create a dummy Response object then fill it with status_code and content. Therefore a Response object will be returned.

JATAYU000 · 2026-01-19T08:01:14Z

CacheMixin._set_cache_response will look at the response object and extract json or xml content from it and save it respectively in .json and .xml files.
CacheMixin._get_cache_response will read these files (.json or .xml) from the given path and create a dummy Response object then fill it with status_code and content. Therefore a Response object will be returned.

This makes a sense now. having an independent download method as I have setup is better than updating requests/caching to return path right?

we can completely remove these lines and always rely on the CacheMixin class. How does that sound?

Yes that would work, but the function definition would be changed i.e. tests etc corresponding to them

geetu040 · 2026-01-19T08:12:51Z

This makes a sense now. having an independent download method as I have setup is better than updating requests/caching to return path right?

I am not sure about that, would require a detailed review

Yes that would work, but the function definition would be changed i.e. tests etc corresponding to them

yes, that is expected

satvshr · 2026-01-20T14:43:34Z

openml/_api/resources/dataset.py

+        path = f"data/features/{dataset_id}"
+        return self.download_file(path)
+
+    def download_qualities_file(self, dataset_id: int) -> Path:


May I know why this function (and similar functions), which are used only in DatasetsV1 and V2, and not called inside functions.py, not private? It becomes hard to distinguish what is actually being called by the SDK and what is being abstracted away.

It is called in functions.py I haven't updated it yet, was still working on it

satvshr · 2026-01-20T14:45:48Z

openml/_api/http/client.py

+        parsed_url = urllib.parse.urlparse(source)
+
+        # expect path format: /BUCKET/path/to/file.ext
+        _, bucket, *prefixes, _file = parsed_url.path.split("/")


_file should be renamed to _ given it is never called, surprised ruff does not call it out.

satvshr · 2026-01-20T14:49:10Z

openml/datasets/functions.py

    Parameters
    ----------
-    data_id : list, optional
+    dataset_id : list, optional


Should be data_id

satvshr · 2026-01-20T14:54:04Z

openml/datasets/functions.py

 def _get_dataset_parquet(
    description: dict | OpenMLDataset,
-    cache_directory: Path | None = None,
+    cache_directory: Path | None = None,  # noqa: ARG001


Why the ARG001 addition, I assume ruff checks were passing before this was added too?

Edit1: Just saw that this is not being used, why not remove it then?

Edit2: This is happening in more than 1 place, I won't mention it in all of them so we can just discuss it here.

Still under work,This method is now updated to not use the param, working on removing it here and update the corresponding test when starting to write tests for this pr, as mentioned in the meet

satvshr · 2026-01-20T14:58:08Z

openml/datasets/functions.py

+    return api_context.backend.datasets.download_dataset_parquet(description, download_all_files)


 def _get_dataset_arff(


@geetu040 This function is not being used yet is still present, because it is being used in tests. Should the test involving this be removed, or should this function be kept? (Same scenario exists in tasks migration too)

Edit: I see this happening in multiple places (_get_dataset_features_file and _get_dataset_qualities_file)

@satvshr refering to the conversation here

There are xml specific parsing logic in OpenMLDataset which should be implemented under the current interfaces for v1/v2 support. The load methods for features and qualities are updated to use the api_context

satvshr · 2026-01-20T14:59:59Z

openml/datasets/functions.py

-    else:
-        raise TypeError("`description` should be either OpenMLDataset or Dict.")
-
-    save_cache_directory = (


I assume cache paths for V1 and V2 are different, hence this code is being removed?

@JATAYU000 I think you missed this

Caching path is removed from here, since handled in the http client, not due to different paths?

satvshr · 2026-01-20T15:53:17Z

openml/datasets/functions.py

    bool
        True if the deletion was successful. False otherwise.
    """
-    return openml.utils._delete_entity("data", dataset_id)


@geetu040 @JATAYU000 I remember being on a call and discussing this, didn't we decide to implement this in the base PR (core) itself and leave it out of our migration implementations? If not, I will need to change tasks.

Yes it was, since it is not yet added I haven't removed this yet

This reverts commit 93155ee.

geetu040

update with #1576 (comment)

JATAYU000 · 2026-03-16T15:02:01Z

@geetu040 There is one issue which is still not address in v2 status update
as we had discussed earlier that the temporary fix (passing api_ke in the url) for passing api_key and json through request needs to be changed and need to figure out a better way to do it.

geetu040 · 2026-03-16T16:04:08Z

as we had discussed earlier that the temporary fix (passing api_ke in the url) for passing api_key and json through request needs to be changed and need to figure out a better way to do it.

I though we were going to use that temporary fix for this PR, since this happens only for a very particular case.

JATAYU000 · 2026-03-16T16:07:03Z

But you mentioned earlier that this is not an optimal fix and we should think of a better solution?

JATAYU000 · 2026-03-19T05:54:46Z

@geetu040
All the test failures are related to minio unable to download in multiple threads? Do you have any idea why this would be happening?

 DEBUG    urllib3.connectionpool:connectionpool.py:544 http://localhost:8000 "GET /minio/datasets/0000/0002/dataset_2.pq HTTP/1.1" 200 31505
WARNING  openml._api.resources.dataset:dataset.py:758 Could not download file from http://localhost:8000/minio/datasets/0000/0002/dataset_2.pq:

Also wwhy are tests on Windows not using Local (OPENML_USE_LOCAL_SERVICES)?

openml-python/.github/workflows/test.yml

Line 179 in 8a5532f

OPENML_TEST_SERVER_ADMIN_KEY: ${{ secrets.OPENML_TEST_SERVER_ADMIN_KEY }}

geetu040 mentioned this pull request Jan 9, 2026

[ENH] V1 → V2 API Migration #1575

Open

25 tasks

geetu040 suggested changes Jan 13, 2026

View reviewed changes

satvshr added 3 commits January 15, 2026 21:58

fixed bugs

65ba66b

fixed bugs

317c6e9

fixed bugs

503ab82

geetu040 mentioned this pull request Jan 16, 2026

[ENH] V1 → V2 API Migration - Tasks #1611

Open

satvshr and others added 4 commits January 16, 2026 15:36

Merge branch 'main' into issue1564

fd7ea2b

bug fixing

fa3cd40

Merge base pr

3ad7268

Download methods

3e7c415

Refactored functions

8933cd8

geetu040 assigned JATAYU000 Jan 19, 2026

satvshr reviewed Jan 20, 2026

View reviewed changes

Update todos, topic endpoints

dfa0ab7

satvshr reviewed Jan 20, 2026

View reviewed changes

geetu040 and others added 16 commits March 6, 2026 10:44

Merge branch 'main' into issue1564

2d452d3

Merge branch 'main' into issue1564

c235812

Trigger CI

39eb823

Merge branch 'main' into migration

50eed37

Merge branch 'main' into issue1564

7a000eb

Merge branch 'main' into issue1564

79f6187

Merge branch 'pr-1577' into migration (merge conflicts)

b1a9e7f

update server methods in config

d716ecf

fix api-version leakage in tests

3c29e71

remove unused migration code

b4ff0b2

debug ci: separate cache for each test-case

93155ee

update port for localhost

d3cc9a7

Revert "debug ci: separate cache for each test-case"

a6b82f4

This reverts commit 93155ee.

rerun CI

3419973

Merge branch 'main' into migration

8de99b7

Merge base migration

d0202b0

geetu040 suggested changes Mar 13, 2026

View reviewed changes

Fix tests for new test setup

0fa9e3b

geetu040 added 3 commits March 16, 2026 20:12

create enum ServerMode

7d61107

update config for ServerMode

1ecbbba

update tests for ServerMode

65472ed

JATAYU000 and others added 5 commits March 17, 2026 15:34

Update status_update

9219266

udpate apikey in _TEST_SERVERS_LOCAL

44b48b5

skip v2 status_update

11b19de

Merge base migration

4df12d3

skip v2 status_update

77d2af2

		return api_context.backend.datasets.download_dataset_parquet(description, download_all_files)


		def _get_dataset_arff(

Uh oh!

Conversation

JATAYU000 commented Jan 8, 2026

Metadata

Uh oh!

codecov-commenter commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JATAYU000 commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issues:

Example:

Uh oh!

geetu040 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JATAYU000 Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geetu040 commented Jan 14, 2026

Uh oh!

JATAYU000 commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geetu040 commented Jan 15, 2026

Uh oh!

JATAYU000 commented Jan 16, 2026

Uh oh!

JATAYU000 commented Jan 19, 2026

Uh oh!

geetu040 commented Jan 19, 2026

Uh oh!

JATAYU000 commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geetu040 commented Jan 19, 2026

Uh oh!

satvshr Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

satvshr Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jan 8, 2026 •

edited

Loading

JATAYU000 commented Jan 9, 2026 •

edited

Loading

geetu040 left a comment •

edited

Loading

JATAYU000 Jan 14, 2026 •

edited

Loading

JATAYU000 commented Jan 15, 2026 •

edited

Loading

JATAYU000 commented Jan 19, 2026 •

edited

Loading

satvshr Jan 20, 2026 •

edited

Loading

satvshr Jan 20, 2026 •

edited

Loading

satvshr Jan 20, 2026 •

edited

Loading

JATAYU000 commented Mar 19, 2026 •

edited

Loading