pymapdl icon indicating copy to clipboard operation
pymapdl copied to clipboard

mapdl.result.filename has mixture of \ and / (windows)

Open natter1 opened this issue 4 years ago • 6 comments

I was looking for an easy way to get an pymapdl result into dpf (see issue). A possible solution would be mapdl.result.filename, but it seems somewhat broken. I get something like:

d:/Al_cantilever/stat\AlCantilever.rst

As a general remark, maybe its possible to use pathlib instead of str-paths (-> discussion at #645)?

natter1 avatar Oct 06 '21 18:10 natter1

Possibly related to #643 and #648. I'll keep an eye out for discrepancies as I work with those and see where this is coming from.

jgd10 avatar Oct 07 '21 09:10 jgd10

@RomanIlchenko1308, can you look at this as well, unless @jgd10 wants to follow up on this.

akaszynski avatar Jan 17 '22 17:01 akaszynski

I'll give this a look tomorrow morning and see how I do. It's been delayed because I'd love to use pathlib everywhere as that seems like a really sensible idea, but it's a much bigger task than just fixing filename. Might be best to fix filename as str and come back to pathlib if it looks too daunting.

jgd10 avatar Jan 17 '22 17:01 jgd10

So, having investigated this I have come to the following conclusions:

  • The discrepancy is caused because the code has a section where it insists that / be used instead of \\ everywhere and makes those replacements, but this does not play nicely with path joining that happens elsewhere, which caused the bug.
  • We can't fix this at the location of those path-joins because they happen in ansys.mapdl.reader and not the pymapdl repo (although another ticket should be made there which I will do).
  • To elaborate further this is because the Result object that mapdl.result returns is an ansys.mapdl.reader object.
  • I can change pymapdl over from string paths to pathlib.Paths but there's a lot to do. I have made a start in the linked PR and intend to do more in due course
  • By shifting over to pathlib.Paths we nullify the first problem entirely and fix this ticket
  • Because pathib.Path plays quite nicely with other string paths it might be ok to merge the changes as we go rather than making one massive PR but that remains to be seen. It would certainly be unconventional, but the PR I've made as it is does fix the bug in this ticket as far as I can tell.

jgd10 avatar Jan 18 '22 13:01 jgd10

We can't fix this at the location of those path-joins because they happen in ansys.mapdl.reader and not the pymapdl repo (although another ticket should be made there which I will do).

We can fix it there as needed. It's an upstream package that we modify and it grows (or changes) in tandem with ansys-mapdl-core.

I can change pymapdl over from string paths to pathlib.Paths but there's a lot to do. I have made a start in the linked PR and intend to do more in due course

That's probably the best solution since it's future focused rather than the patchwork of fixes we currently have. Recommend using pathlib.Path within ansys-mapdl-reader first, then fixing it here.

akaszynski avatar Jan 18 '22 15:01 akaszynski

I should have clarified. I meant we can't fix it in this repo*, since the changes need to happen in the other one, but otherwise yes.

jgd10 avatar Jan 18 '22 15:01 jgd10