restructure package #11
Conversation
add checks to check if sphinx build
There was a problem hiding this comment.
Looks good, thanks!
Just some minor comments / fixes.
Before removing the datastructures, I'm not sure if we still maybe want to keep them even thought they're not used. Just as templates? Or alternative, they can just be moved into the documentation? @axelwalter, what do you think?
For entry points, it's a personal preference, but I would prefer if they were all lower case.
| if backend == "bokeh": | ||
| try: | ||
| module = importlib.import_module("pyopenms_viz.plotting._bokeh") |
There was a problem hiding this comment.
This needs to be updated, remove the .plotting since it's no longer a submodule directory
| elif backend == "matplotlib": | ||
| try: | ||
| module = importlib.import_module("pyopenms_viz.plotting._matplotlib") |
There was a problem hiding this comment.
This needs to be updated, same reason as above.
| elif backend == "plotly": | ||
| try: | ||
| module = importlib.import_module("pyopenms_viz.plotting._plotly") |
There was a problem hiding this comment.
This needs to be updated, same reason as above.
There was a problem hiding this comment.
yes will fix this, however it is strange that the plotting still seems to work with this bug?
| "MSBokeh = pyopenms_viz._bokeh", | ||
| "MSPlotly = pyopenms_viz._plotly", | ||
| "MSMatplotlib = pyopenms_viz._matplotlib", |
There was a problem hiding this comment.
I'm personally not in favour of using a mixed-case for the entry points. I prefer using snake_case, I think it's also just easier, so a user doesn't have to worry/think about whether certain letters need to be capitalized or not.
|
About the datastructures. If I understand correctly all columns which affect the plot are specified by the user now (e.g. x, y, by columns). And I will do the same for columns such as custom peak color in the spectrum plot. I like this approach which does not assume any columns are named correctly. If that is the case everywhere I would vote for removing the datastructures. |
Cleanup package - remove old code and simplify organization
This will make autogenerating the API for the docs easier