Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]file_name argument in methods #61

Closed
4 tasks
Tian-Jionglu opened this issue Oct 15, 2020 · 8 comments
Closed
4 tasks

[BUG]file_name argument in methods #61

Tian-Jionglu opened this issue Oct 15, 2020 · 8 comments
Assignees

Comments

@Tian-Jionglu
Copy link
Contributor

Tian-Jionglu commented Oct 15, 2020

Describe the bug
It's a bug found in my application with pycatia, to create TIFF drawing from CATDrawing.
https://github.com/Tian-Jionglu/pycatia/projects/1#card-47338604

To Reproduce

from pycatia import catia

caa = catia()
drawing_doc = caa.active_document

# dir "F:/temp/output/" is existed, dir "F:/output/" is not. "test.CATDrawing" is the active_document

# case1
drawing_doc.export_data("F:/temp/output/", "tif", overwrite=True)
    # expect to get file "F:/temp/output/test.tif". Result is com_error.

# case2
drawing_doc.export_data("F:/temp/output", "tif", overwrite=True)
    # expect to get file "F:/temp/output/test.tif". Get file "F:/temp/output.tif" as a result.

# case3
drawing_doc.export_data(f"F:/temp/output/{drawing_doc.name}", "tif", overwrite=True)
    # expect to get file "F:/temp/output/test.tif". Result is nothing

# case4
drawing_doc.export_data("F:/temp/output/test.tif", "tif", overwrite=True)
    # expect to get file "F:/temp/output/test.tif". Result is as expected.

# case5
drawing_doc.export_data("F:/temp/output/test", "tif", overwrite=True)
    # expect to get file "F:/temp/output/test.tif". Result is as expected.

# case6
drawing_doc.export_data("F:/temp/output/test.name", "tif", overwrite=True)
    # expect to get file "F:/temp/output/test.name.tif". Result is nothing.(".name" is unknown suffix, sometimes users type tihs by fault)

# case7
drawing_doc.export_data("F:/output/test", "tif", overwrite=True)
    # expect an error to remind the directory is not exist. Result is com_error

Expected behavior
see notations.

Desktop (please complete the following information):

Additional context
It's a similar bug as in #58 .
Type str or Path, the file_name argument should be?
I think the type can be uniformed in methods below:

  • docuemnt.export_data
  • document.save_as
  • sheet.print_to_file
  • maybe there is more...
@Tian-Jionglu
Copy link
Contributor Author

A commit added to fix this issue.
More consideration is needed for question "Type str or Paht?".

@Tian-Jionglu
Copy link
Contributor Author

One more commit added to make document.export_data work proper with relative path 55c9ab6

@Tian-Jionglu
Copy link
Contributor Author

document.save_as do the same change via commit bbf8749

@Tian-Jionglu
Copy link
Contributor Author

I found another solution for this issue, as the clause in documents.open:
file_name = os.path.abspath(file_name)

With relative directory supported, "Example 8" can be simpler.

#! /usr/bin/python3.6

"""
    Example 8:
    Open all CATParts in source directory and save to IGS in target directory.
"""

import os
import glob
from pycatia import CATIADocHandler

# make these directories the full pathname.
source_directory = 'tests/cat_files'
target_directory = '__junk__'

# if full paths are supplied above you should not do this.
# source_directory = os.path.join(os.getcwd(), source_directory)
# target_directory = os.path.join(os.getcwd(), target_directory)
# clauses above are no longer needed 

# This loop assumes there are NO sub-directories.
# only convert CATParts.

part_doc_pattern = os.path.join(source_directory , '*.CATPart')
files = glob.glob(part_doc_pattern)

for file in files:
    with CATIADocHandler(file) as handler:
        part_doc = handler.document
        part_doc.export_data(target_directory, 'igs', overwrite=True)

@evereux
Copy link
Owner

evereux commented Oct 25, 2020

I've made my own changes here but have tried to incorporate what you've said. Please take a look. :-)

I don't like the use of Path.absolute() it's undocumented and I don't know if it's behavior will change or be dropped altogether for some older versions?

So, I've used the documented Path.is_absolute() to log a warning telling the user they should probably use absolute paths. I don't think pycatia should try and guess at the users intent too much.

Sorry for the delay. I've not had a chance to test what I've done here using your examples but will try and get on that this week. If com_errors are still raised we can at least provide better error handling -> messages for weird cases.

Thanks as always dude. 👍

@Tian-Jionglu
Copy link
Contributor Author

Commit c14f338 to fix bug in former commit.

Tian-Jionglu pushed a commit to Tian-Jionglu/pycatia that referenced this issue Nov 19, 2020
Tian-Jionglu pushed a commit to Tian-Jionglu/pycatia that referenced this issue Nov 19, 2020
Tian-Jionglu pushed a commit to Tian-Jionglu/pycatia that referenced this issue Nov 19, 2020
@Tian-Jionglu
Copy link
Contributor Author

Tian-Jionglu commented Nov 26, 2020

Please think about this comment.a25762e#r44547011

@evereux
Copy link
Owner

evereux commented Dec 27, 2020

I've replied again to that comment.

Aside from that is there anything from preventing this issue being closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants