Conversation
The algorithm is identical between NUBASE and AME so a minor update to also take the name means we can share it.
A few major changes here: - Inheritance was not the correct technique to use so stop doing that and use the static Converter methods and make the parameter locations in the datafile a member so we can still access. - Instead of a large match case on the year to set the majority of the members to the same value most of the time, create a dataclass to store the values and tweak as required for each year by creating a new instance. - The use of the dataclass also allows us to set the column names at the same time as the parameter start and end line positions and automate the creation of other variables. Ideally we would remove all manual setting of anything related to columns, but we often do different things with different subsets so it's not clear how achieve this.
See f190918 for details.
See f190918 for details
See f190918 for details
The utils/ directory was created to allow the separation of functionality but everything ended up in a single class for ease. Before the repo grows too large let's split up as planned.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #29 +/- ##
==========================================
- Coverage 99.73% 99.55% -0.18%
==========================================
Files 12 15 +3
Lines 749 451 -298
==========================================
- Hits 747 449 -298
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#27
This started as a refactor and splitting of the Converter class into more appropriate entities but there has been some scope creep.
When setting the start and end positions for each parameter in the various data files across the years, we used a large match case statement setting all values each time, unless they were identical between years. We have now changed this and for each file type, created a storage dataclass with either the initial file layout, or the one most common as the default and we override the necessary values on construction for the specific year. We have also changed the naming convention of the start and end variables to match the column names used in the dataframe so we can dynamically create column limits and other required containers.
The user will not see any changes, but there has been a significant reshuffle of files and their contents at the core of the parsing.