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

Refactor section inputs #80

Merged
merged 13 commits into from
Nov 23, 2021

Conversation

amoodie
Copy link
Contributor

@amoodie amoodie commented Nov 17, 2021

Refactored the inputs to sections (closes #34)

  • PathSection
  • StrikeSection
  • DipSection
  • CircularSection
  • RadialSection

Each section can take inputs as either dimensional coordinates or as indices. Each argument has a paired argument xxxx_idx that is the input variables. Documentation and tests updated,

**warning: this is a breaking change. The behavior of some input arguments has been maintained with a deprecation warning (e.g., y as input for a strike section), but others could not maintain backwards compatability (e.g., radius for a circular section now specified coordinates instead of indices and will likely result in an error without updating your own code).

Note: this PR also fixed a few issues with sections. Namely, Path and Radial sections would occasionally be drawn the wrong direction or have incorrect ordering.

Will also complete and closes #33

@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #80 (c2d39de) into develop (7d3210d) will increase coverage by 0.60%.
The diff coverage is 94.78%.

❗ Current head c2d39de differs from pull request most recent head 820c2fa. Consider uploading reports for the commit 820c2fa to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #80      +/-   ##
===========================================
+ Coverage    86.85%   87.46%   +0.60%     
===========================================
  Files            9        9              
  Lines         2762     2912     +150     
===========================================
+ Hits          2399     2547     +148     
- Misses         363      365       +2     
Impacted Files Coverage Δ
deltametrics/cube.py 78.04% <ø> (ø)
deltametrics/plot.py 90.04% <ø> (ø)
deltametrics/section.py 95.11% <94.71%> (-0.63%) ⬇️
deltametrics/utils.py 73.99% <100.00%> (+3.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d3210d...820c2fa. Read the comment docs.

Andrew Moodie added 4 commits November 17, 2021 15:42
…aining backwards comapatability. The new options are 'distance', 'idx', and 'length', where distance is the distance from the dim1 wall in coordinates to draw the section, idx is the distance from the dim1 wall in cell indices to draw the section, and length is a two-element tuple specifying the bounding dim2 coordinates for the section in the same convention as either idx or distance (both idx and distance cannot be specified).
…the 'CircularSection' to incorporate the new dimensional parameters 'radius' and 'origin', and their counterparts in indices 'radius_idx' and 'origin_idx'.
@amoodie amoodie force-pushed the refactor_section_inputs branch from 0a8484f to 87dbe49 Compare November 18, 2021 20:06
@amoodie amoodie marked this pull request as ready for review November 22, 2021 23:25
@amoodie
Copy link
Contributor Author

amoodie commented Nov 22, 2021

@elbeejay can you build the docs and read through the changes to the Section API description? Just want to make sure you agree with all the names and changes before merging. Don't expect you to dig into the code, unless you want to.

@amoodie amoodie mentioned this pull request Nov 23, 2021
5 tasks
deltametrics/section.py Outdated Show resolved Hide resolved
deltametrics/section.py Outdated Show resolved Hide resolved
@elbeejay
Copy link
Contributor

@elbeejay can you build the docs and read through the changes to the Section API description? Just want to make sure you agree with all the names and changes before merging. Don't expect you to dig into the code, unless you want to.

Just read through the docs and they look really good. The names and everything feel very consistent and are intuitive I think.

@amoodie
Copy link
Contributor Author

amoodie commented Nov 23, 2021

@elbeejay sweet, thank you. The documentation is so fragile to formatting 😩

reminder, this will be a breaking change if you pull after merge!

@amoodie amoodie merged commit bd5a4df into sandpiper-toolchain:develop Nov 23, 2021
@amoodie amoodie deleted the refactor_section_inputs branch March 20, 2022 21:46
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.

Make the Section types work with the coordinates of the arrays from xarray Create a DipSection section type
2 participants