alan-turing-institute / deepsensor

A Python package for tackling diverse environmental prediction tasks with NPs.
https://alan-turing-institute.github.io/deepsensor/
MIT License
72 stars 15 forks source link

Minor issue in `Documentation > User-guide > Task': Missing code cell #114

Closed kimbente closed 2 months ago

kimbente commented 4 months ago

Thanks for the amazing package + incredible documentation!

I am finally trying out DeepSensor and spotted a minor issue in the docs:

In https://alan-turing-institute.github.io/deepsensor/user-guide/task.html#the-deepsensor-task, just before

print(task)

the following code cell is not showing:

from deepsensor.data import TaskLoader task_loader = TaskLoader(context=[era5_ds, land_mask_ds], target=station_df) task = task_loader("2016-06-25", context_sampling=[52, 112], target_sampling=245)

Cheers, Kim

tom-andersson commented 4 months ago

Thanks for the kind words @kimbente!

This is actually deliberate, because by this point in the user guide the reader has not come across the TaskLoader class yet. We have the line “Before diving into the TaskLoader class which generates Task objects from xarray and pandas objects, we will first introduce the Task class itself”.

IMO it would be confusing to see that code cell at this point in the docs, but I can see how more curious readers might wonder where the Task object came from.

You can see that remove-cell was hard-coded into the cell’s metadata, which tells Jupyter Book not to render it: https://github.com/alan-turing-institute/deepsensor/blob/125fb25483fc47a58add9edc6196a75e23579cd6/docs/user-guide/task.ipynb#L125

Do you have any suggestions? We could either leave this as-is, or replace that line with hide-cell (so it can be expanded), or make it more clear in the paragraph above.

kimbente commented 2 months ago

I thought it was by mistake, which is why I raised it, but I should have assumed it was intentional... apologies.

Personally I would prefer it to be a hide-cell, so it is clear that it was defined somewhere previously. I think that would also match cohesion and the logical style in the rest of the documentation.

Good either way!