-
Notifications
You must be signed in to change notification settings - Fork 0
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 horizontal error bars and linear regression #11
base: main
Are you sure you want to change the base?
Conversation
paul019
commented
Sep 4, 2023
- change parser including parser logic (see README)
- update README accordingly
- update sample data accordingly
- update Measurement class to hold errors for x- and y-value
- update drawer to draw horizontal error bars
- change parser including parser logic (see README) - update README accordingly - update sample data accordingly - update Measurement class to hold errors for x- and y-value - update drawer to draw horizontal error bars
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a nice new feature, thanks for implementing this @paul019. Here are my two cents ;)
def get_linear_regression(self): | ||
return do_linear_regression(self.measurements) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too sure if transformer should be worried with the linear regression. Based on what scope we have right now for the transformer
Transforms measurements into PDF coordinates based on grid configuration. Note that analyze_and_offset_measurements must be called first.
I'd say that a linear regression does not fit in this scope. Maybe construct a linear regression directly in drawer.py
in line 48
? Not sure if this is a good idea though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it quite fitting for the transformer. Then the drawer is only worried with drawing and the transformer is there for any data processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some more nitpicky stuff
One thing to mention: we should probably somehow note (e.g. in the release notes) that this is a breaking change since the |
…aul019/pappe into feauture/horizontal-error-bars
Formerly, data and offset data was interchanged at some point. This bug fix also makes the language used clearer.