Add initial stable API with tests

sci-visus / OpenVisus

Open Source distribution of the ViSUS capabilities

Other

27 stars 18 forks source link

Closed klacansky closed 1 year ago

klacansky commented 1 year ago

Any progress on merging it? Thanks!

scrgiorgio commented 1 year ago

Sorry if I am late on this. My suggestions:

please git pull / git rebase since there were bugs with ARCO vs IDX caching (and cache_dir too) that have been fixed in recent commits. All changes should not conflict with your modifications.
do not rename create_idx to create_dataset but create an "alias". There is code around (external to this repo, in particular in Jupyter Notebooks) that uses create_idx. In general, always prefer non-breaking API changes.
do not add the Dataset python class to dataset.py. There is already a Dataset class in C++ and having the same name in Python can be confusing (or difficult to handle with SWIG-generated code), in particular in the case of "from OpenVisus import *".

klacansky commented 1 year ago

I renamed the function and dataset, and also updated the test.

scrgiorgio commented 1 year ago

All ok. Don't get the name "Dataset1" (usually I prefer self-describing names, what does "1" stands for?)

klacansky commented 1 year ago

Direct3D uses that for versioning, so when there are changes, you return Dataset2, Dataset3, ...

What name would you prefer?

klacansky commented 1 year ago

Can you push a new tag please? Thanks!

scrgiorgio commented 1 year ago

Just pushed.

I was thinking about adding a file "Libs/swig/stable_api" so people can do:

from OpenVisus.stable_api import Dataset,LoadDataset # etc...

this way we can have name overlapping. (could be stable_api_v<n> as well)

klacansky commented 1 year ago

Interesting. I would then make the experimental API like (so that most users have the simple import OpenVisus as ov).

from OpenVisus.experimental_api import Dataset,LoadDataset # etc...