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

Improve api docs for qcodes.dataset #1660

Merged
merged 10 commits into from
Aug 14, 2019

Conversation

lakhotiaharshit
Copy link
Contributor

@lakhotiaharshit lakhotiaharshit commented Aug 8, 2019

Changes proposed in this pull request:

  • Fixing the docstrings of the user-facing API docs

Currently finished parts:
qcodes.dataset

  • qcodes.dataset.measurements
  • qcodes.dataset.plotting
  • qcodes.dataset.data_set
  • qcodes.dataset.database_extract_runs
  • qcodes.dataset.legacy_import

Workflow:
@GateBuilder and I will be working on parts of user-facing API docs each week. The finished parts will be mentioned on the top.

@WilliamHPNielsen
@jenshnielsen
@Dominik-Vogel
@astafan8

@codecov
Copy link

codecov bot commented Aug 8, 2019

Codecov Report

Merging #1660 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1660   +/-   ##
=======================================
  Coverage   66.94%   66.94%           
=======================================
  Files         144      144           
  Lines       17804    17804           
=======================================
  Hits        11918    11918           
  Misses       5886     5886

@lakhotiaharshit lakhotiaharshit changed the title Fixing the user-facing api docs [WIP] Fixing the user-facing api docs Aug 12, 2019
@lakhotiaharshit lakhotiaharshit changed the title [WIP] Fixing the user-facing api docs Fixing the user-facing api docs Aug 12, 2019
Copy link
Collaborator

@jenshnielsen jenshnielsen left a comment

Choose a reason for hiding this comment

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

Hi guys. Looks good. A large amount of this is not user facing so it has lower priority.

What is userfaceing is defined by the sphinx configuration https://qcodes.github.io/Qcodes/api/index.html

@@ -476,7 +476,7 @@ def run_timestamp(self, fmt: str = "%Y-%m-%d %H:%M:%S") -> Optional[str]:
started. If the run has not yet been started, this function returns
None.

Consult with `time.strftime` for information about the format.
Consult with ``time.strftime`` for information about the format.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest making this a clickable link. We have intersphinx loaded so you should be able to do

:func:`time.strftime`

but I have not tested it

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should work. Changed accordingly.

the data of this `DataSet`. The definition can be found at
`subscription.subscribers`.
the data of this :class:`.DataSet`. The definition can be found at
``subscription.subscribers``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add something like in the qcodesrc.json config file

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this makes sense. Thanks.

@@ -158,7 +158,7 @@ def _extract_single_dataset_into_db(dataset: DataSet,
target_exp_id: int) -> None:
"""
NB: This function should only be called from within
:meth:extract_runs_into_db
:py:meth:`extract_runs_into_db`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure the :py: is needed? The python env is the default and it is not used on any of the other constructs. (class func etc.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think this is necessary, but I saw it has been used in the docstrings in e.g. data_set.py (see the docstring get_data_as_pandas_dataframe()). I will remove it, but also the existing ones should be handled in an other PR if this is unnecessary.

Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

Thanks!

@jenshnielsen jenshnielsen changed the title Fixing the user-facing api docs Improve api docs for qcodes.dataset Aug 14, 2019
@jenshnielsen jenshnielsen merged commit f160cc2 into microsoft:master Aug 14, 2019
@lakhotiaharshit lakhotiaharshit deleted the userfacing_api_docs branch August 20, 2019 14:30
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.

None yet

4 participants