-
Notifications
You must be signed in to change notification settings - Fork 4
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
Major Code Refactor: Clean Up Parameters #39
Open
jcharkow
wants to merge
70
commits into
OpenMS:main
Choose a base branch
from
jcharkow:refactor_ui_apply_manual
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
first attempt incorperatro refactor_ui changes into _core change MSPlot to dataclass and made appropriate changes set up reliance on config (SpectrumConfig, ChromatogramConfig...)
get rid of arguments and kwargs, instead store in the class. Makes it less confusing as to what need to call what with now for the most part just need to call things with "self"
this makes the interface more readable when we are combining multiple plots into a single figure. E.g. marginal plots e.g. mirror spectrum plot
figure or axes is a class variable (dependent on backend) plot() method does not return anything --> legends directly created from plot
to clean up what has to be passed allow for all variables to be part of config except for the dataframe because that is what it is called on. working for bokeh chromatogram currrently
rename figure to canvas for consistency across backends for bokeh since canvas is a figure object make self.fig and self.canvas linked
having difficulties currently with tooltips
change marker size
bug relative intensity was not being applied, update snapshot. also update axis labels
found bug in the old snapshot with coloring that fixed
spectrum tests are stange so re-design them
new snapshots look reasonable so update them
update chromatogram snapshots because now it uses the linewidth argument
plotly tooltips index does not seem reproducible with the "by" column so remove this from the chromatogram plot and update snapshots.
remove index from tooltips
manual looking and they look fine
note: had an error where when using a binning of "auto" and "sturges" or binning of True (e.g. whenever use the pd.cut method) it seems that the inveral object is never really gotten rid of properly and this causes an error in plotly graphing. Could not figure out the error so switched to using numpy
seems that previous updates were not saved? or just very minor differences
for spectrum strangely the files produced were .json instead of .html and this is because the plotly backend was used by mistake. Fix this by fixing the conftest.py bug snapshots verified manually and look good
seems that bokeh snapshots currently do not get checked correctly. Will have to look into this. however bokeh snapshots seem to be working as expected
update doc to reflect new config
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Major code refactor, organize parameters better in _config file.
Format code as dataclasses for cleaner implementation
Remove kwargs popping