Closed nick-falco closed 5 years ago
@nick-iris , in the newhelp branch (https://github.com/PIC-IRIS/PH5/tree/newhelp), I demonstrated that you could define a description-supplying class for each module, so that the definition would reside in each module, and could be updated along with the module as it evolves, so that the new/modified definitions would be called from the individual modules rather than from a separate file/class .
It seems like what you are describing is a single help module with a class containing definitions and entry points for all of the PH5 commands. As these individual modules evolved, the class definitions would have to be updated in this single file, perhaps replacing the current ph5.utilities.ph5_help.py module. Is that correct? Thanks, Dave
In my opinion, the only thing typing ph5
should do is provide a list of all available commands along with a short description for each. The method to receive help information for each individual command would remain the same.
Currently users have no way of knowing what ph5 commands are available without going to the wiki page. The goal was that the ph5
entry point would provide a way of exposing the list of commands from the application.
My proposed design was to create a new module (e.g. entry_points.py
) containing a dictionary with each command name, description, and entry point (e.g. ph5.utilities.125a2ph5:main). This new module would get imported by the setup.py
module as well as a ph5_help.py
module residing in the same top level directory. This way there is one master list of entry points for ph5 that is used for both the setup.py
configuration and to provide a list of available commands when a user enters ph5
.
I'm not opposed to improving the help information that is in each module, but I think it could still be documented using argparse. For example entering ph5tostationxml --help
would continue to display that help documentation for the ph5tostationxml
command.
That makes sense. I don't want to alter the --help information for each command.
I'll make a new branch and set about adding an entry_points.py module, and getting ph5_help to import and make use of that. Once that's done, it shouldn't be too hard to import the new info into setup.py to replace the hard-coded list there now.
Thinking about this some more, I think I'd move the ph5_help.py
module (possibly renamed to help.py
) to the ph5 directory next to the LICENSE.txt
and CONTRIBUTOR.txt
files. Also I'd add the entry_points.py
module to that same directory. This will make the import of the entry_points
module from the setup.py
cleaner. Otherwise you'd have to add a __init__.py
module in the root folder which is undesirable.
Sounds good.
@nick-iris , I've made some headway on the new help approach. Please check out https://github.com/PIC-IRIS/PH5/tree/newhelp I haven't altered setup.py yet, will need some help doing that. The basic functionality is there. New file entry_points.py (in same directory as setup.py) creates a dictionary with keys = names of scripts; values are (1) simple description, (2) entry points for use by setup.
In the same directory, I made a new file, help.py, which imports entry_points and prints the short descriptions in alpha-order.
You can test it by typing $ python help.py
If this approach looks solid, I could use some help modifying the setup.py file to both use the entry points defined in the dictionary, and to alias help.py so that when user types $ ph5, the list of scripts and definitions is provided.
I should mention that while files ph5/clients/ph5toevt.py and ph5/clients/ph5view/ph5_viewer.py are marked as changed, I reverted them back to their original state (they had been the site of a previous approach demonstration). So the only new files are entry_points.py and help.py, and the change log.
@dthomas1953, thanks for sharing. I think I'd change things around slightly. Please see below:
I'd move the entry_points.py
module down a level under the ph5
directory next to theLICENSE.txt
and CONTRIBUTOR.txt
files. It makes the imports work out better.
I'd also change the contents of entry_points.py
to something like this:
class EntryPoint():
def __init__(self, command, entry_point, description):
self.command = command
self.entry_point = entry_point
self.description = description
def get_entry_point_str(self):
return "{} = {}".format(self.command,
self.entry_point)
class CommandList():
def __init__(self):
self.entrypoints = {
'gui_scripts':[
EntryPoint('experiment_t_gen',
'ph5.utilities.changes:startapp',
'A GUI for building the experiment summary '
'kitchen-exchange format (kef) file'),
EntryPoint('kefedit',
'ph5.utilities.kefedit:startapp',
'A GUI for opening, editing, and saving .kef '
'and .ph5 files'),
EntryPoint('noven',
'ph5.utilities.noven:startapp',
'A GUI for converting CSV metatdata files into '
'kef files for PH5'),
EntryPoint('pforma',
'ph5.utilities.pformagui:startapp',
'A GUI for loading MetaData into PH5'),
EntryPoint('ph5view',
'ph5.utilities.ph5_viewer:startapp',
'A GUI program for plotting responses, saving '
'files to SEGY, etc'),
],
... etc. etc.
Here I've defined a class that called EntryPoint
that contains the command name, command description, and entry point. This method has one instance method called get_entry_point_str
that returns the string that is used in the setup.py
entry point dictionary.
In setup.py
you could then just do this:
from ph5.entry_points import CommandList
command_list = CommandList()
setup(
....
entry_points = {group: [ep.get_entry_point_str() for ep in eps]
for group, eps in command_list.entrypoints.items()}
....
)
I also noticed some python semantics being broken in the code you uploaded that I thought I'd point out. One thing is that class names should always be capitalized. Another is that your class declaration was missing parentheses.
Like the entry_points.py
module, I'd also move the help.py
module down a level under the ph5 directory next to the LICENSE.txt
and CONTRIBUTOR.txt
files. You'll also need to update it to work with the new enty_points.py
module.
Sounds good, I'll make some updates. More later...
@nick-iris I think I got it working as you envisioned it, please give it a try... https://github.com/PIC-IRIS/PH5/tree/newhelp/ph5
@dthomas1953 Thanks for sharing. I made a few edits to the presentation of the the help message, but I think everything looked good otherwise. Let me know what you think of the new help page.
OK, I'll check it out.
The list of commands that gets listed is fairly extensive, and I suspect that many of the commands within PH5 rarely are used. I wonder if we should trim the list down to commonly used commands, and then only display all commands when users specifically requests to see them. For example we could only display all commands if one enters ph5 --all
. Thoughts?
Sounds reasonable.
Are you familiar enough with the process of creating a PH5 volume from scratch that you could identify what commands are important? Also feel free to reclassify commands into different groups if you think they could be better organized. I just followed the way Derick classified the commands in the wiki.
I will consult with the Data Group to see which commands are most important. I'll add an arg-parse to the script to accommodate the --all option. More later, Dave
Try it now, @nick-iris I got the Data Group's input on which commands to show iff -a or --all option is used.
@dthomas1953 Thanks! Overall this looks good. I made a few minor edits to and cleaned up the code a bit, but nothing major.
There were a few coding issues that I noticed, listed below:
Your python changes contained indentation with a mix of spaces and tabs. Python is very fussy when it comes to indentation. I suggest configuring your IDE to treat tabs as "4 spaces", this way you can still use tabs for convenience when writing your code. You can do this very easily in the Eclipse IDE, and I suspect others support this feature as well.
There were a number of places where there were lines of code commented out (i.e. commented out print statements). It is generally best practice to not comment out code. It's better to remove the code entirely, since old code will be tracked in source control if you ever were to need it again.
I'll have a look, run flake8 etc. Was mainly checking suitability before doing the formal pull request.
Looks like you cleaned it up nicely. I changed the version date, will do the pull request,
Is your feature request related to a problem? Please describe. Currently when a user types
ph5
they get the following message:The problem is that all the supplemental help for each command is hardcoded in a separate module (
/ph5/utilities/ph5_help.py
) that is independent from the actual command source code. Additionally the listing of commands is hardcoded.Describe the solution you'd like I think this should be simplified. IMO, when a user types
ph5
then a list of available commands should displayed along with a short description for each. For example:You could just define a class containing each command name, description, and entry point (e.g.
ph5.utilities.125a2ph5:main
). This could then dynamically get used to define the list of commands in thesetup.py
definition and in the help message.