-
Notifications
You must be signed in to change notification settings - Fork 127
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
Documentation fixes and formatting #1095
Conversation
also fixed broken links and bumped some package versions
renamed env to `docs-lint`, changed arxiv link to abstract instead of pdf, and various other fixes
linux build is failing on the minimal conf so I'm adding more lines back in. hard to debug locally because the mac build has a different set of warnings.
this syncs up the names on the manuals page and the API experiments page
Removed FakeService and FakeJob since they're more for internal testing and not useful for most users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@@ -143,5 +156,14 @@ | |||
BlochTrajectoryAnalysis, | |||
) | |||
|
|||
from .data_processing import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these all added because they are referenced in the documentation? @nkanazawa1989 is it okay to promote these all to curve_analysis
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular it's mean_xy_data
that's referenced in the average_method
analysis option, the others may not be referenced. We should also consider renaming curve_analysis/data_processing.py
to avoid name confusion with the data processor module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I am not sure I fully understand what needs to be curve_analysis/data_processing.py
versus qiskit_experiments.data_processing
🙂 It seems like only multi_mean_xy_data
, filter_data
, and data_sort
are imported out of data_processing.py
, so I wonder if we want all of these functions to be public, though we need to if we are referring to the docstring (maybe the discussion should be moved).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
I am still not sure all those curve_analysis functions need to be documented for outside use, but I don't feel strongly.
Summary
This PR is essentially the documentation fixes from #1080 since the lint CI pipeline needs more work.
Details and comments
add_data
curve_analysis/data_processing.py
toutils.py