#1180 Fix output file extension handling and normalize extensions to lowercase #1510
Conversation
…custom check logic
… domain custom check logic" This reverts commit c76daa6.
| return f"{base_path}{expected_ext}" | ||
|
|
||
| path_lower = output_path.lower() | ||
| known_extensions = [".json", ".xlsx", ".xls"] |
There was a problem hiding this comment.
Could you please move these to the constants folders and import from there. Placing it in init file should be good enough.
RamilCDISC
left a comment
RamilCDISC
left a comment
There was a problem hiding this comment.
I ran a validation using following command:
python core.py validate -s sdtmig -v 3-2 -of JSON -o /user/reports/result -dp tests/resources/test_datasets.xlsx
According to readme this should write a report in /us/reports folder named result.json. Right now when i executed it on mac system i get following error:
[████████████████████████████████████] 100% Traceback (most recent call last): File "/Users/ramilabbasov/Desktop/ISSUES/ISS-1180/cdisc-rules-engine/core.py", line 915, in <module> cli() File "/Users/ramilabbasov/Desktop/ISSUES/venv/lib/python3.12/site-packages/click/core.py", line 1157, in __call__ return self.main(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/ramilabbasov/Desktop/ISSUES/venv/lib/python3.12/site-packages/click/core.py", line 1078, in main rv = self.invoke(ctx) ^^^^^^^^^^^^^^^^ File "/Users/ramilabbasov/Desktop/ISSUES/venv/lib/python3.12/site-packages/click/core.py", line 1688, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/ramilabbasov/Desktop/ISSUES/venv/lib/python3.12/site-packages/click/core.py", line 1434, in invoke return ctx.invoke(self.callback, **ctx.params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/ramilabbasov/Desktop/ISSUES/venv/lib/python3.12/site-packages/click/core.py", line 783, in invoke return __callback(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/ramilabbasov/Desktop/ISSUES/venv/lib/python3.12/site-packages/click/decorators.py", line 33, in new_func return f(get_current_context(), *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/ramilabbasov/Desktop/ISSUES/ISS-1180/cdisc-rules-engine/core.py", line 466, in validate run_validation( File "/Users/ramilabbasov/Desktop/ISSUES/ISS-1180/cdisc-rules-engine/scripts/run_validation.py", line 204, in run_validation reporting_service.write_report() File "/Users/ramilabbasov/Desktop/ISSUES/ISS-1180/cdisc-rules-engine/cdisc_rules_engine/services/reporting/json_report.py", line 72, in write_report os.makedirs(output_dir, exist_ok=True) File "<frozen os>", line 215, in makedirs File "<frozen os>", line 225, in makedirs OSError: [Errno 30] Read-only file system: '/user'
I looked into the error. I believe the issue is likely the path /user/reports/result. On macOS, user directories are actually located under /Users/username/, not /user/. So, the command is trying to write to a folder at the system root that likely doesn't exist or is read-only. On Windows, path handling is different (it might interpret the slash differently relative to the current drive), which is why I couldn't replicate the exact crash, but the path is technically invalid there as well. To address this, I’ve updated the error handling. Now, instead of a raw system crash, it will catch the permission error and show a clear message asking for a valid path. I also updated the README to use a relative path (reports/result) to avoid this confusion across different OSs. Since I can't test the macOS scenario directly, could you try these two steps?
|
RamilCDISC
left a comment
RamilCDISC
left a comment
There was a problem hiding this comment.
- Reviewing the PR for any unwanted code or comments.
- Reviewing the PR logic in accordance with AC.
- Ensuring all unit and regression testing pass.
- Running manual validations on CLI and rule editor.
- Validating following edge cases:
- no output format and output path mentioned
- valid output format specified with no output path.
- valid output format with valid output path.
- invalid output format with valid output path.
- invalid output format with invalid output path.
- multiple same output formats specified.
- multiple different output formats.
- directory specified as an output path.
Fixes issue #1180 where using -of JSON with -o filename.json would create filename.json.json. The fix prevents double extensions by checking if the output path already has the correct extension before appending, and replaces known extensions when they don't match the output format. Additionally, the output message now displays the actual filenames created instead of the original user input, and file extensions are normalized to lowercase for consistency across platforms. Parent directories are now automatically created when they don't exist.