esmf-org / esmf-profiler

ESMF Profiler application converts binary traces into a web based, dynamic report.
0 stars 2 forks source link

name argument error #127

Closed MinsukJi-NOAA closed 2 years ago

MinsukJi-NOAA commented 2 years ago

Describe the bug For v0.1.1 on Hera, the name argument gives error.

To Reproduce Steps to reproduce the behavior:

  1. Local Install esmf-profiler on Hera by following README.md steps 1 through 7
  2. Invoke the command esmf-profiler -t /scratch1/NCEPDEV/stmp2/Minsuk.Ji/FV3_RT/rt_7393/cpld_control_p7_64/traceout -n "cpld_control_p7_64" -o ./out_dir
  3. See error:
    (venv) [Minsuk.Ji@hfe09 esmf-profiler]$ git branch
    * (HEAD detached at v0.1.1)
    development
    (venv) [Minsuk.Ji@hfe09 esmf-profiler]$ esmf-profiler -t /scratch1/NCEPDEV/stmp2/Minsuk.Ji/FV3_RT/rt_7393/cpld_control_p7_64/traceout -n "cpld_control_p7_64" -o ./out_dir
    usage: esmf-profiler [-h] -t TRACEDIR -n NAME -o OUTDIR [-p PUSH] [-v] [-s]
    esmf-profiler: error: argument -n/--name: invalid _alphanumberic value: 'cpld_control_p7_64'
ryanlong1004 commented 2 years ago

@rsdunlapiv this is built into the code.

name allows alpha-numeric characters.

Do you want to update this to allow spaces, dashes, etc?

MinsukJi-NOAA commented 2 years ago

@ryanlong1004 I thought this was a bug because some examples in the README file use underscores. For example: esmf-profiler -t ./fresh_traces -n "build_abc123" -o "output" -s

rsdunlapiv commented 2 years ago

@ryanlong1004 the only reason for limiting the name is because it is used to create the paths when pushing to github.io - e.g., <username>/<profilename>

So I see no issue in allowing more characters, we just need to be sure that whatever the user provides can be used to create a valid path that can be pushed.

ryanlong1004 commented 2 years ago

@MinsukJi-NOAA This will go out on the next release.

Accepting any character in the below regex now: r"^[\w\-. ]+$"