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.

Sign up for GitHub

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

Jump to bottom

top-level import shortcuts #184

Merged
merged 8 commits into from
Jun 23, 2021
Merged

top-level import shortcuts #184

merged 8 commits into from
Jun 23, 2021

Conversation

prjemian
Copy link
Contributor

fix #183

@prjemian prjemian added this to the v1.0.0 milestone Jun 18, 2021
@prjemian prjemian requested a review from mrakitin June 18, 2021 18:23
@prjemian prjemian self-assigned this Jun 18, 2021
@prjemian
Copy link
Contributor Author

@mrakitin : Additional work here would be to use the new shortcuts in all the documentation and examples. First, let's review the implementation. This is motivated by similar shortcuts in ophyd.

@prjemian prjemian marked this pull request as draft June 18, 2021 18:28
@prjemian
Copy link
Contributor Author

note: Converted to draft since additional work was indicated above.

mrakitin
mrakitin approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Looks good! Added a small suggestion to the test.

hkl/tests/test_init.py Outdated Show resolved Hide resolved
@prjemian prjemian marked this pull request as ready for review June 18, 2021 20:04
@prjemian
Copy link
Contributor Author

@mrakitin -- Would you please look at the additional changes to documentation and examples. The only remaining imports that were not given shortcuts were the various hkl.calc.CalcGEOMETRY. I do not believe those are so popular that we should add to the namespace if someone writes from hkl import *.

@prjemian prjemian requested a review from mrakitin June 18, 2021 21:25
@prjemian
Copy link
Contributor Author

Setting this back to draft, waiting on the changes from PR #186 (hkl.diffract.Constraint moves to hkl.util.Constraint) which will need to merge here.

@prjemian prjemian marked this pull request as draft June 19, 2021 00:56
@prjemian prjemian marked this pull request as ready for review June 19, 2021 01:02
@prjemian prjemian marked this pull request as draft June 19, 2021 01:03
@prjemian prjemian mentioned this pull request Jun 19, 2021
Merged
@prjemian
Copy link
Contributor Author

@mrakitin - Thanks for the review! Can you take a look at PR #186? I realized it's a blocker for this issue since the unit tests won't run here due to the circular import to be resolved in #186.

@mrakitin
Copy link
Member

@prjemian, done :)

@prjemian prjemian mentioned this pull request Jun 23, 2021
Closed
@prjemian
Copy link
Contributor Author

Moved the documentation tasks into new issue so this work can be closed and #187 can be finalized for review.

@prjemian prjemian marked this pull request as ready for review June 23, 2021 16:20
@prjemian prjemian merged commit ea6ea29 into main Jun 23, 2021
@prjemian prjemian deleted the 183-import-shortcuts branch June 23, 2021 16:28
prjemian added a commit that referenced this pull request Jun 25, 2021
@prjemian
MNT #184
55573a1
prjemian added a commit that referenced this pull request Jun 25, 2021
@prjemian
TST #184
4b61925
@prjemian prjemian mentioned this pull request Jul 30, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrakitin mrakitin mrakitin approved these changes

Assignees

@prjemian prjemian

Labels
enhancement
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.

import shortcuts for popular items
2 participants
@prjemian @mrakitin