Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Added rename project option #1013

Closed
wants to merge 2 commits into from
Closed

Added rename project option #1013

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 30, 2022

  • Added a rename projection option to the gmrecords proj subcommand.

  • Replaced all uses of os.path with Pathlib in gmrecords.py

  • Replaced all uses of os.path with Pathlib in projects.py

  • Updated the changelog to reflect these changes

Replaced all uses of os.path with Pathlib in gmrecords.py
Replaced all uses of os.path with Pathlib in projects.py
Updated the changelog to reflect these changes
@gferragu
Copy link
Collaborator

gferragu commented Oct 4, 2022

@smithj382 have you been able to make any sense of the path errors for Windows? I was looking over the azure logs and trying to figure out what's up. It seems like a relative path issue still

@@ -140,8 +143,7 @@ def main(self, gmrecords):
if not prompt.query_yes_no(question, default="yes"):
sys.exit(0)

shutil.rmtree(conf_path, ignore_errors=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emthompson-usgs are things at data_path no longer being removed here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose not. Perhaps we need to fix that, but I think it is a separate issue. I'm note sure the "delete" argument ever gets used by anyone.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gferragu I modified the shutil to delete the parent of config_path so that it would also delete the project folder. See ) I don't know if this is the right approach (probably not) so if the alternative way of deleting the config_path and data_path folders separately is preferred I can change this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no guarantee that the the data and conf directories share a common parent directory (although that is the default), which is why it done separately before.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emthompson-usgs OK I didn't think about this because I am using the default. I can revert it back, but I think I'll wait until the Windows relative_path issue is resolved.

@ghost
Copy link
Author

ghost commented Oct 4, 2022

@smithj382 have you been able to make any sense of the path errors for Windows? I was looking over the azure logs and trying to figure out what's up. It seems like a relative path issue still

@gferragu I tried to revert to not using relative_to for the Windows platform (as the build that passed Azure was doing), but it seems like the test used relative_to for Windows despite the if platform != "Windows": use and I'm not sure why.

@emthompson-usgs
Copy link
Member

Yes, the purpose of the if-statement was to not use relative paths on Windows and it had fixed things before. I think that the reason for the errors may be unrelated to this, but it is difficult to test/debug without a Windows system to run it on.

@gferragu
Copy link
Collaborator

gferragu commented Oct 4, 2022

@smithj382 Yeah that seems weird, it's definitely still running the relative_to bit despite running on Windows

@gferragu
Copy link
Collaborator

gferragu commented Oct 4, 2022

This Python issue about relative_to looks pretty similar to what we are seeing in the Azure logs here.

@emthompson-usgs
Copy link
Member

These edits were included in #1016 in which I fixed the Windows issue.

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

Successfully merging this pull request may close these issues.

3 participants