DOC: Add more docstrings with some renaming #712

mferrera · 2024-06-27T08:42:46Z

Resolves #711
Works on #696

This PR suppresses the large number of warnings that were occurring when documentation was built as well as transfers some of the documentation that existed in datamodel.rst as docstrings.

Resolves #711 because the errors emitted will slowly disappear as more docstrings are added to the the items listed in every toctree page.

mferrera · 2024-06-27T09:08:32Z

src/fmu/dataio/datastructure/meta/meta.py

@@ -114,18 +114,28 @@ class User(BaseModel):
    )


-class FMUCase(BaseModel):
+class Case(BaseModel):


I removed some of these prefix FMU on the class names because I felt it should be clear that we are coming from FMU

mferrera · 2024-06-27T09:09:53Z

src/fmu/dataio/datastructure/meta/meta.py

@@ -341,23 +398,25 @@ class Context(BaseModel):
    stage: enums.FmuContext


-class FMUClassMetaCase(BaseModel):
+class FMUCaseAttributes(BaseModel):


An exception about the FMU prefix is here, because we are speaking explicitly about the fmu: block in the data model.

Would appreciate feedback for possibly better names for this class

mferrera · 2024-06-27T09:10:35Z

src/fmu/dataio/datastructure/meta/meta.py



-class FMUClassMetaData(BaseModel):
+class FMUAttributes(FMUCaseAttributes):


FMUAttributes is derived from the simpler FMUCaseAttributes, where case of course corresponds to the reduced metadata included in just case metadata

Seems logical

"of course"... :)

mferrera · 2024-06-27T09:11:30Z

src/fmu/dataio/datastructure/meta/meta.py

@@ -393,27 +452,47 @@ def __get_pydantic_json_schema__(
        return json_schema


-class ClassMeta(BaseModel):
+class MetadataBase(BaseModel):


Renamed this to reflect that the fields in this class are the base top-level fields in our current two metadata cases: case metadata and object metadata

mferrera · 2024-06-27T09:11:45Z

src/fmu/dataio/datastructure/meta/meta.py

+class CaseMetadata(MetadataBase):
+    """The FMU metadata model for an FMU case."""


Is there a better name for this?

Not that I can think of

The "case" concept is tricky. It is highly conventional, but over the years I guess it has become so institutionalized that it is difficult to get away from. ERT however has no concept of the "case". There is the concept of "experiment", but these are not 1:1. It is also a bit chaotic with many different implementation of a corresponding variable across different FMU workflows. The word "case" is also used to describe different things. E.g. some would refer to realizations as "cases" and so on. So there is some confusion.

We could consider adding a description here of what we mean with the term "case" in this context, perhaps in a note or something. This is the definition we have included in the Sumo docs:

A case represent a set of iterations that belong together, either by being part of the same run (i.e. history matching) or by being placed together by the user, corresponding to /scratch/<asset>/<user>/<my case name>/.

Updated with this comment both here and on the Case model specifically

mferrera · 2024-06-27T09:11:58Z

src/fmu/dataio/datastructure/meta/meta.py

+class ObjectMetadata(MetadataBase):
+    """The FMU metadata model for a given data object."""


Is there a better name for this?

mferrera · 2024-06-27T09:12:58Z

src/fmu/dataio/datastructure/meta/meta.py

-    case: FMUCase
-    model: FMUModel
+    case: Case
+    model: Model


Model may not be an improvement over FMUModel, from the code perspective, but it matches the schema field name

jcrivenaes

It seems OK with me, but I would like to have an opinion from the other reviewers

perolavsvendsen · 2024-06-28T07:19:31Z

src/fmu/dataio/datastructure/meta/meta.py

+    """The name of the iteration. This is typically reflecting the folder name on
+    scratch. In ERT, custom names for iterations are supported, e.g. "pred". For this
+    reason, if logic is implied, the name can be risky to trust - even if it often
+    contains the ID, e.g. "iter-0"


Perhaps this should be rephrased - it is actually the id which is difficult to trust. I can run a case with 3 ensembles named "pred1", "pred2" and "pred3" and they will all be given fmu.iteration.id: 0 I believe.

Currently all logic is put on fmu.iteration.name when consuming data.

I just cut it off after the ~~second~~ third sentence so it doesn't become stale documentation, but am happy to add additional context if you think it should be there

src/fmu/dataio/datastructure/meta/meta.py

perolavsvendsen

I think I need to see the end result in ReadTheDocs. Perhaps we can just push it, and iterate from there? Or does this have other implications that just docs?

perolavsvendsen · 2024-06-28T07:35:50Z

src/fmu/dataio/datastructure/meta/meta.py

+class CaseMetadata(MetadataBase):
+    """The FMU metadata model for an FMU case."""


The "case" concept is tricky. It is highly conventional, but over the years I guess it has become so institutionalized that it is difficult to get away from. ERT however has no concept of the "case". There is the concept of "experiment", but these are not 1:1. It is also a bit chaotic with many different implementation of a corresponding variable across different FMU workflows. The word "case" is also used to describe different things. E.g. some would refer to realizations as "cases" and so on. So there is some confusion.

We could consider adding a description here of what we mean with the term "case" in this context, perhaps in a note or something. This is the definition we have included in the Sumo docs:

A case represent a set of iterations that belong together, either by being part of the same run (i.e. history matching) or by being placed together by the user, corresponding to /scratch/<asset>/<user>/<my case name>/.

src/fmu/dataio/datastructure/meta/meta.py

perolavsvendsen · 2024-06-28T07:40:08Z

src/fmu/dataio/datastructure/meta/meta.py



-class FMUClassMetaData(BaseModel):
+class FMUAttributes(FMUCaseAttributes):


"of course"... :)

mferrera · 2024-06-28T08:01:30Z

@perolavsvendsen it looks like this. This will only affect documentation. (User shows a bit weird here because that documentation has not been docstring'd yet)

DOC: Suppress duplicate obj description warnings

fede792

mferrera self-assigned this Jun 27, 2024

mferrera marked this pull request as ready for review June 27, 2024 09:06

mferrera requested review from tnatt, jcrivenaes and perolavsvendsen June 27, 2024 09:06

mferrera commented Jun 27, 2024

View reviewed changes

mferrera force-pushed the add-doc-strings branch 2 times, most recently from 1ef496c to 95c105f Compare June 27, 2024 10:20

jcrivenaes approved these changes Jun 28, 2024

View reviewed changes

perolavsvendsen reviewed Jun 28, 2024

View reviewed changes

src/fmu/dataio/datastructure/meta/meta.py Outdated Show resolved Hide resolved

perolavsvendsen reviewed Jun 28, 2024

View reviewed changes

src/fmu/dataio/datastructure/meta/meta.py Outdated Show resolved Hide resolved

perolavsvendsen reviewed Jun 28, 2024

View reviewed changes

src/fmu/dataio/datastructure/meta/meta.py Outdated Show resolved Hide resolved

mferrera force-pushed the add-doc-strings branch 3 times, most recently from bb354aa to 03c147e Compare June 28, 2024 07:44

perolavsvendsen approved these changes Jun 28, 2024

View reviewed changes

mferrera force-pushed the add-doc-strings branch from 03c147e to 81f321a Compare June 28, 2024 07:54

DOC: Add more docstrings with some renaming

5221978

mferrera force-pushed the add-doc-strings branch from 81f321a to 5221978 Compare June 28, 2024 08:10

mferrera merged commit b1b52d1 into equinor:main Jun 28, 2024
13 checks passed

mferrera deleted the add-doc-strings branch June 28, 2024 08:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Add more docstrings with some renaming #712

DOC: Add more docstrings with some renaming #712

mferrera commented Jun 27, 2024

mferrera Jun 27, 2024

mferrera Jun 27, 2024

mferrera Jun 27, 2024 •

edited

Loading

jcrivenaes Jun 28, 2024

perolavsvendsen Jun 28, 2024

mferrera Jun 27, 2024 •

edited

Loading

mferrera Jun 27, 2024

jcrivenaes Jun 28, 2024

perolavsvendsen Jun 28, 2024

mferrera Jun 28, 2024

mferrera Jun 27, 2024

mferrera Jun 27, 2024

jcrivenaes left a comment

perolavsvendsen Jun 28, 2024

mferrera Jun 28, 2024 •

edited

Loading

perolavsvendsen left a comment

perolavsvendsen Jun 28, 2024

perolavsvendsen Jun 28, 2024

mferrera commented Jun 28, 2024



		class FMUClassMetaData(BaseModel):
		class FMUAttributes(FMUCaseAttributes):

		class CaseMetadata(MetadataBase):
		"""The FMU metadata model for an FMU case."""

		class ObjectMetadata(MetadataBase):
		"""The FMU metadata model for a given data object."""

DOC: Add more docstrings with some renaming #712

DOC: Add more docstrings with some renaming #712

Conversation

mferrera commented Jun 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mferrera Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mferrera Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcrivenaes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mferrera Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

perolavsvendsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mferrera commented Jun 28, 2024

mferrera Jun 27, 2024 •

edited

Loading

mferrera Jun 27, 2024 •

edited

Loading

mferrera Jun 28, 2024 •

edited

Loading