ParasolJS / parasol-es

ES6 module for interactive visualization of multi-objective optimization problems
https://parasoljs.github.io/
MIT License
26 stars 5 forks source link

Proposed API Name Change #9

Closed wraseman closed 6 years ago

wraseman commented 6 years ago

Hey Josh,

Proposed change of ps.removeSelection() and ps.keepSelection() to ps.removeData() and ps.keepData()

Take a look at the comments I made on the README on the develop branch (search: "TODO"). I feel that some of the names for the API are misleading. For instance, I like the naming of ps.exportData() [i.e., Parasol.exportData()]. It is clear what is does, and in the arguments the user can specify what type of data to export (brushed, marked, all, selected). However, ps.removeSelection(), for instance, is a bit confusing. To a new user, this might seem equivalent to ps.resetSelection(). ps.resetSelection() makes sense to me because the method is geared toward resetting selections (marks or brushes). On the other hand, ps.removeSelection() is changing the data that is being visualized, not the selections.

Long story short, I think it would make sense to change ps.removeSelection() to ps.removeData() (and ps.keepSelection to ps.keepData). That would be consistent with how ps.exportData() is used and differentiates them from ps.resetSelection().

Proposed change of ps.exportData(selection, filename) to ps.exportData(selection, filename, exportAll=false)

Another aspect that is confusing is the selection argument for export data. The options for this argument are brushed, marked, both, or all. "All" doesn't really make sense with the other options because it isn't actually an argument. I think we should add another argument called exportAll that, if set to true, would override the selection argument and just export all the data. That way the selection argument would be the same across the other aspects of the API and we could still include the "all" option which is definitely valuable.

Thoughts? Billy

joshhjacobson commented 6 years ago

I think this makes more sense, I'm on board. Once I implement this change, I'll close the issue.