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

add Coils and magnetic fields to docs api #615

Merged
merged 7 commits into from
Aug 11, 2023
Merged

Conversation

dpanici
Copy link
Collaborator

@dpanici dpanici commented Aug 7, 2023

No description provided.

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #615 (3f51e10) into master (6adcb4c) will increase coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #615      +/-   ##
==========================================
+ Coverage   94.30%   94.32%   +0.01%     
==========================================
  Files          77       77              
  Lines       17696    17696              
==========================================
+ Hits        16689    16691       +2     
+ Misses       1007     1005       -2     
Files Changed Coverage Δ
desc/coils.py 98.83% <ø> (ø)

... and 3 files with indirect coverage changes

@dpanici
Copy link
Collaborator Author

dpanici commented Aug 8, 2023

black format the doc code

Copy link
Collaborator

@ddudt ddudt left a comment

Choose a reason for hiding this comment

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

update example code to have black formatting

@dpanici dpanici requested a review from ddudt August 9, 2023 19:14
docs/api.rst Outdated Show resolved Hide resolved
docs/api.rst Outdated Show resolved Hide resolved
normal=[0, 0, 1],
r_n=R_coil,
modes=[0],
grid=LinearGrid(N=100),
Copy link
Member

Choose a reason for hiding this comment

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

FYI, I'm not sure about having grid objects assigned to each coil. I was considering getting rid of it in #583 but felt we should discuss it first. I think it would be better to add something like source_grid as an optional argument to compute_magnetic_field, especially since after #583 whatever grid you assign to the coil won't be used by the underlying Curve.compute method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, that is something we can change in #494 when I update the coil stuff to reflect the #583 changes. This PR though is just for the docs

@dpanici dpanici merged commit cc10a94 into master Aug 11, 2023
@dpanici dpanici deleted the dp/add_fields_to_api branch August 11, 2023 12:55
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