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 Echoregions #91

Closed
9 tasks done
ctuguinay opened this issue Jun 9, 2023 · 0 comments · Fixed by #95
Closed
9 tasks done

Refactor Echoregions #91

ctuguinay opened this issue Jun 9, 2023 · 0 comments · Fixed by #95
Assignees
Labels
enhancement setup infrastructure or packaging

Comments

@ctuguinay
Copy link
Collaborator

ctuguinay commented Jun 9, 2023

Due to the current organization of echoregions being somewhat convoluted and assuming too much about how many functionalities each of the subclasses for Lines and Echoregions2D share, we need to do some refactoring of the current structure.

The new structure will be as the following:

image

The tasks will be as follows:

  • Implement refactored regions2d/regions2d.py
  • Implement refactored 'regions2d/api.py`
  • Implement refactored regions2d/parser.py
  • Implement refactored lines/lines.py
  • Implement refactored 'lines/api.py`
  • Implement refactored lines/parser.py
  • Implement refactored 'utils/io.py`
  • Implement refactored utils/time_utils.py
  • Refactor import in tests to match the refactoring that has happened in /echoregions
@ctuguinay ctuguinay added enhancement setup infrastructure or packaging labels Jun 9, 2023
@ctuguinay ctuguinay self-assigned this Jun 9, 2023
@ctuguinay ctuguinay moved this to Todo in Echoregions Jun 9, 2023
@ctuguinay ctuguinay moved this from Todo to Done in Echoregions Jun 23, 2023
@ctuguinay ctuguinay mentioned this issue Jun 22, 2023
@ctuguinay ctuguinay linked a pull request Jun 26, 2023 that will close this issue
@ctuguinay ctuguinay moved this from Done to Review Needed in Echoregions Jun 30, 2023
ctuguinay added a commit that referenced this issue Jul 2, 2023
### Refactoring

This PR closes the following issues: #91, #88, #83, #43.

**The main folders are now:**
- utils
- lines
- regions2d

**As opposed to before:**
- utils
- convert
- formats
- mask
- plot

**Refactoring notes:**
- `core.py` contains `read_evl` and `read_evr`.
- In both regions2d and lines, plotting is still kept as a class function: `r2d.plot() `and `lines.plot()`.
- In both regions2d and lines, masking is still kept as a class function: `r2d.mask() `and `lines.mask()`.
- In both regions2d and lines, parsing is kept outside of class: `parse_regions_file(input_file)` and `parse_lines_file(input_file)`.
- We now always initialize regions2d and lines with appropriate .evr and .evl functions at initialization ALWAYS. This is enforced with an error if not initialized properly (incorrect .evr or .evl file).

**Other Changes Include:**
- Added line masking based on Valentina's example notebook with options for what to do with NA values and what interpolation method to use.
@github-project-automation github-project-automation bot moved this from Review Needed to Done in Echoregions Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement setup infrastructure or packaging
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant