Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conf tree #22

Merged
merged 16 commits into from
May 19, 2020
Merged

Conf tree #22

merged 16 commits into from
May 19, 2020

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Apr 29, 2020

Sibling PRs:

Highlights:

  • Autodocument Cylc/Parsec configurations from the new ConfigNode tree thing.
  • Add a powerful relative referencing syntax.

Examples:

This feature is configured using the options in :cylc:conf:`file.rc[foo][bar][baz]`:

.. cylc-scope: file.rc[foo][bar][baz]

(no need to specify the full path in this section because we have set the scope)

* :cylc:conf:`pub` - does something
* :cylc:conf:`aux` - does something else
* :cylc:conf:`foot` - and now for something completely different

.. cylc:scope::

(but we do have to remember to reset the scope afterwards)

Documentation:

All [auto-]documented in the cylc_lang extension, try building the docs 😀.

@oliver-sanders oliver-sanders added this to the 1.0.1 milestone Apr 29, 2020
@oliver-sanders oliver-sanders self-assigned this Apr 29, 2020
This was referenced Apr 29, 2020
@oliver-sanders
Copy link
Member Author

Tests failing due to dependency on cylc/cylc-flow#3559


.. rst-example::

.. auto-cylc-conf:: my-conf2.rc
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't auto-document from JSON any more.

types = {}
for obj in objects:
types.update(obj)
for name, info in sorted(types.items()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for name, info in sorted(types.items()):
for _, info in sorted(types.items()):

My pylint is complaining about unused varible name - pycodestyle seems happy though! Happy to ignore.

@@ -195,59 +211,84 @@ def tokens_from_partials(partials):


def tokens_relative(base, override):
"""
"""Return one pat h relative to the other.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Suggested change
"""Return one pat h relative to the other.
"""Return one path relative to the other.

ret = {**base}
# ensure that base is an aboslute path
if not base.get('conf'):
return ValueError(f'{base} is not an absoute path')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return ValueError(f'{base} is not an absoute path')
return ValueError(f'{base} is not an absolute path')

"""
flag = False
ret = {**base}
# ensure that base is an aboslute path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# ensure that base is an aboslute path
# ensure that base is an absolute path

# The following is a minimal domain for documenting parsec objects:

def parsec_ref(tokens):
"""The detokenise equivalent for parsec (much simpler).i
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""The detokenise equivalent for parsec (much simpler).i
"""The detokenise equivalent for parsec (much simpler).

Think this 'i' can go, unless I'm mistaken?

try:
docname = self.get(tokens)
except KeyError:
# object does not exist, "nitpicky" mode will pick this up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"nitpicky" mode - like that :)



def test_doc_section(simple_section):
"""It should docuement configuration sections."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""It should docuement configuration sections."""
"""It should document configuration sections."""

@datamel
Copy link
Contributor

datamel commented May 1, 2020

So, I have started my review. Only a few typos from what I have read through so far. I still have to look at the sibling prs and double check some of the logic for my own sanity. Pytests are passing for me locally. I still have to look at the integration. Looks great so far!

Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still need to test by building but I’ve read through code, only a few little things I’ve noticed so far.

cylc/sphinx_ext/cylc_lang/domains.py Outdated Show resolved Hide resolved
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have built it, changes look great. Suggested changes to minor typos.

@oliver-sanders oliver-sanders requested a review from kinow May 18, 2020 15:03
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with the cylc-doc PR.

@kinow kinow merged commit 1e91907 into cylc:master May 19, 2020
@oliver-sanders oliver-sanders deleted the conf-tree branch May 20, 2020 09:29
@oliver-sanders oliver-sanders modified the milestones: 1.0.1, 1.1.0 Jun 8, 2020
@oliver-sanders oliver-sanders mentioned this pull request Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants