Experimental docs.rs changes #181

aldanor · 2021-10-24T11:27:10Z

Enable doc_cfg for nightly builds (on docs.rs and locally if enabled)
(Experimental) Replace hdf5_1_2_3 config flags with feature = "1.2.3". This will produce annotations like "Enabled on crate feature 1.10.0 only" in the docs instead of "Enabled on hdf5_1_10_0 only". If I understand correctly, this will also allow the users to refer to these as hdf5/1.10.0 externally (?). Features can be named as "hdf5-1.10.0" but that would look a bit ugly in the docs (with the numbers only, it looks pretty neat, see below for examples).
(TODO) we should definitely build the docs on the latest HDF5 version available (1.12.1?).
(TODO) ideally, we want mpio/blosc/lzf on docs.rs as well (the latter two are already there).
Should h5_have_parallel and such be renamed to feature = "have-parallel"?

Note: support for period for feature names has been only recently added to cargo (circa 1.50).

Related: #141.

aldanor · 2021-10-24T11:28:23Z

Example 1:

Example 2:

Example 3:

mulimoen · 2021-10-24T11:57:45Z

I added a commit to move cfg to internal, and added to Blosc item to get the feature flag to show

aldanor · 2021-10-24T12:01:20Z

Cool, thanks - I've noticed the issue with LocationToken as well. The docs will need a lot of reshuffling (e.g. doc(no_inline) in many places, etc), we'll sort it out eventually.

So what do you think about feature = "1.2.3" in general?

aldanor · 2021-10-24T12:01:40Z

(Also note the question about other h5_* flags, I've updated PR description)

mulimoen · 2021-10-24T12:01:52Z

The hdf5 crate feature versions are a bit odd. Any way of making clear this is the c-library and this is the minimum version? min-h5c-1.2.3?

mulimoen · 2021-10-24T12:06:15Z

I think version numbers alone are fine if we document this on the entry page of docs.rs and in the Cargo.toml (in case people are looking there)

mulimoen · 2021-10-24T12:36:30Z

Some observations:

enums might depend on a feature (LocationInfo and Filter). Should we mark these as #[non_exhaustive]?
DatasetBuilderData and friends do not apply the feature flag correctly and does not mark items conditionally available.
SimpleExtents has some oddities regarding Deref<Target = [Extent]>, items are shown as This is supported on non-test only.

mulimoen · 2021-10-24T12:42:17Z

We are not using much new functionality from 1.2 yet, we could mark LocationType::TypeMap with #[cfg_attr(docrs, doc(cfg(feature = "1.12.0")))] to get the same effect as building on hdf5-1.12.1. With the next breaking change we could bump hdf5-src
`

mulimoen · 2021-10-24T12:44:10Z

h5_have_parallel vs have_parallel: Either we keep the h5 prefix and do version number as h5_1.12.0 or we document in src/lib.rs and Cargo.toml for minimal confusion

aldanor · 2021-10-24T12:50:03Z

For DatasetBuilder*, it's definitely an overlooked bug - methods should be feature gated (if it's due to paste macro, then the impl_builder! should gain another variant where a minimum version is passed in so it applies #[cfg()]).

Agreed re: keeping numbers alone and mentioning it in the docs / Cargo.toml / changelog.

Re: h5_have_parallel -> I'd probably vouch for renaming to have-parallel (from outside, it's hdf5/have-parallel then), and, similarly, mentioning it in the manifest/docs.

Re: SimpleExtents, not sure, that sure looks weird! Any ideas?

Feature-dependent enums should probably be marked as non-exhaustive, indeed.

mulimoen · 2021-10-24T13:32:19Z

Re: SimpleExtents, not sure, that sure looks weird! Any ideas?

Could be a std problem. I also see it for

doc/hdf5/types/struct.VarLenUnicode.html
doc/hdf5/types/struct.VarLenArray.html
doc/hdf5/types/struct.FixedAscii.html
doc/hdf5/types/struct.FixedUnicode.html
doc/hdf5/types/struct.VarLenAscii.html
doc/hdf5/struct.SimpleExtents.html
doc/hdf5/struct.Hyperslab.html

And another variant for Hyperslab.html#method.to_vec for non-no_global_oom_handling

aldanor · 2021-10-24T13:59:55Z

#![doc(cfg_hide(no_global_oom_handling))] perhaps?

mulimoen

Most items should now be feature gated such that they appear correctly on docs.rs. Marking enums as non_exhaustive, would a breaking change and should wait until the next major version.

aldanor · 2021-11-12T12:36:22Z

Cargo.toml

@@ -16,6 +16,10 @@ default = []
 mpio = ["mpi-sys", "hdf5-sys/mpio"]
 lzf = ["lzf-sys", "errno"]
 blosc = ["blosc-sys"]
+# The features with version numbers such as 1.10.3, 1.12.0 are metafeatures
+# and is only available when the HDF5 library is at least this version.
+# Features have_direct and have_parallel are also metafeatures and dependent


@mulimoen have_direct and have_parallel are not features as of now. Should we make them features as well? I.e., feature = "have-direct" and feature = "have-parallel", so that they show up the same way as versions?

Oh yeah, that is of course better, I assumed they would show up, but it seems not

aldanor · 2021-11-12T12:36:56Z

build.rs

-            "DEP_HDF5_HAVE_PARALLEL" => "h5_have_parallel".into(),
-            "DEP_HDF5_HAVE_THREADSAFE" => "h5_have_threadsafe".into(),
-            "DEP_HDF5_MSVC_DLL_INDIRECTION" => "h5_dll_indirection".into(),
+            "DEP_HDF5_HAVE_DIRECT" => "have_direct".into(),


Wonder, should we have _ -> - and make all of these feature=?

aldanor · 2021-11-12T12:39:02Z

src/hl/dataset.rs

@@ -907,8 +907,12 @@ macro_rules! impl_builder {
            self.builder.$name($($var),*); self
        }
    };
-    ($plist:ident: $name:ident($($var:ident: $ty:ty),*)) => {
+    ($(#[cfg(feature = $feature:literal)])*


Just wondering, wouldn't something like #[cfg($tt:tt)] work, so you wouldn't have to hard-code specific syntax here?

I think I tried, but this was not as straight forward as I thought. My experience with macros is too limited to see what went wrong in this instance

aldanor · 2021-11-12T12:41:56Z

(Was away for a little while, back now)

Wonder, for the docs.rs build, will we be able to get all the have-parallel, have-direct, etc entries show up? (and what can we do in order to make it work?)

aldanor · 2021-11-12T18:14:08Z

hdf5-sys/src/h5s.rs

@@ -112,7 +115,7 @@ extern "C" {
    ) -> htri_t;
 }

-#[cfg(any(hdf5_1_12_0, hdf5_1_10_7))]


@mulimoen Btw, what's this?... Shouldn't this be equivalent to just 1.10.7?

Yes, that is equivalent. The function was backported so I kept hdf5_1_12_0 for simplicity. It is superfluous now

aldanor · 2021-11-12T21:26:10Z

@mulimoen Looks good to merge? (I'll probably update the changelog a bit)

We still need to figure how to build docs on:

1.12.1
direct/parallel/threadsafe/mpio/blosc/lzf all enabled

(but that's a separate issue)

mulimoen · 2021-11-12T21:45:03Z

Enabled this for hdf5-sys now. It's not ideal everywhere, some items such as hbool_t is suggested as only available with certain features enabled even though it is always present. This is very minor compared to seeing which functions are available in the current version. Looks good to merge now

aldanor added 2 commits October 24, 2021 11:33

Enable doc_cfg on doc.rs builds

42268f1

Replace hdf5_1_2_3 cfg flags with feature="1.2.3"

b810b31

Add cfg to document item availability

ef7ee8c

Add docs.rs like CI task

2d6dc4c

mulimoen added 2 commits October 26, 2021 16:57

Add doc(cfg) for 1.12 item

e6b966c

Remove h5_ prefix on internal features

2e71218

mulimoen force-pushed the feature/doc-cfg branch from 34fdc70 to 2e71218 Compare October 26, 2021 17:32

mulimoen added 2 commits October 26, 2021 17:42

Add documentation regarding meta-features

75a4aa5

Add features inside macro expansion to show in doc

1bbede2

mulimoen approved these changes Oct 28, 2021

View reviewed changes

aldanor commented Nov 12, 2021

View reviewed changes

aldanor added 2 commits November 12, 2021 18:09

hdf5-sys: h5_have_* -> feature = "have-*"

79caf1d

hdf5 crate: cfg(have_*) -> feature="have-*"

a3faa4c

aldanor commented Nov 12, 2021

View reviewed changes

Clean up impl_builder!() macros a little bit

c8431ca

Enable doc cfg for hdf5-sys

4ccf8bb

aldanor merged commit 994f33a into master Nov 12, 2021

aldanor deleted the feature/doc-cfg branch November 12, 2021 21:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental docs.rs changes #181

Experimental docs.rs changes #181

aldanor commented Oct 24, 2021 •

edited

Loading

aldanor commented Oct 24, 2021

mulimoen commented Oct 24, 2021

aldanor commented Oct 24, 2021

aldanor commented Oct 24, 2021

mulimoen commented Oct 24, 2021

mulimoen commented Oct 24, 2021

mulimoen commented Oct 24, 2021 •

edited

Loading

mulimoen commented Oct 24, 2021 •

edited

Loading

mulimoen commented Oct 24, 2021

aldanor commented Oct 24, 2021

mulimoen commented Oct 24, 2021 •

edited

Loading

aldanor commented Oct 24, 2021

mulimoen left a comment •

edited

Loading

aldanor Nov 12, 2021 •

edited

Loading

mulimoen Nov 12, 2021

aldanor Nov 12, 2021

aldanor Nov 12, 2021

mulimoen Nov 12, 2021

aldanor commented Nov 12, 2021

aldanor Nov 12, 2021

mulimoen Nov 12, 2021

aldanor commented Nov 12, 2021 •

edited

Loading

mulimoen commented Nov 12, 2021

Experimental docs.rs changes #181

Experimental docs.rs changes #181

Conversation

aldanor commented Oct 24, 2021 • edited Loading

aldanor commented Oct 24, 2021

mulimoen commented Oct 24, 2021

aldanor commented Oct 24, 2021

aldanor commented Oct 24, 2021

mulimoen commented Oct 24, 2021

mulimoen commented Oct 24, 2021

mulimoen commented Oct 24, 2021 • edited Loading

mulimoen commented Oct 24, 2021 • edited Loading

mulimoen commented Oct 24, 2021

aldanor commented Oct 24, 2021

mulimoen commented Oct 24, 2021 • edited Loading

aldanor commented Oct 24, 2021

mulimoen left a comment • edited Loading

Choose a reason for hiding this comment

aldanor Nov 12, 2021 • edited Loading

Choose a reason for hiding this comment

mulimoen Nov 12, 2021

Choose a reason for hiding this comment

aldanor Nov 12, 2021

Choose a reason for hiding this comment

aldanor Nov 12, 2021

Choose a reason for hiding this comment

mulimoen Nov 12, 2021

Choose a reason for hiding this comment

aldanor commented Nov 12, 2021

aldanor Nov 12, 2021

Choose a reason for hiding this comment

mulimoen Nov 12, 2021

Choose a reason for hiding this comment

aldanor commented Nov 12, 2021 • edited Loading

mulimoen commented Nov 12, 2021

aldanor commented Oct 24, 2021 •

edited

Loading

mulimoen commented Oct 24, 2021 •

edited

Loading

mulimoen commented Oct 24, 2021 •

edited

Loading

mulimoen commented Oct 24, 2021 •

edited

Loading

mulimoen left a comment •

edited

Loading

aldanor Nov 12, 2021 •

edited

Loading

aldanor commented Nov 12, 2021 •

edited

Loading