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

grass.temporal.stds_import: Use pathlib Path.read_text and Path.write_text for proj string #4236

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

echoix
Copy link
Member

@echoix echoix commented Aug 27, 2024

Similar changes for stds_export are not in this PR by purpose. During the weekend, I had made a mistake in stds_export somewhere and didn't find it, but the mismatch in lengths of init file vs list file in stds_import, that wasn't touched yet. That's why I insist so much on separating input/output in different PRs.

Details on a failing test saved because stds_import and stds_export weren't changed together

image

In the spirit of #4224 (comment), this is only fixes the part where a context manager can be avoided.

See PR description of #4234 for an illustration of the implementation of read_text() and write_text()

@echoix echoix requested a review from ninsbl August 27, 2024 02:07
@github-actions github-actions bot added Python Related code is in Python libraries labels Aug 27, 2024
@wenzeslaus
Copy link
Member

Details on a failing test saved because stds_import and stds_export weren't changed together

That's strange. That should not matter. There might be some end of line counting issue, but maps should be counted, not lines. I didn't see that though with just a quick look into the source code.

@echoix
Copy link
Member Author

echoix commented Aug 27, 2024

Details on a failing test saved because stds_import and stds_export weren't changed together

That's strange. That should not matter. There might be some end of line counting issue, but maps should be counted, not lines. I didn't see that though with just a quick look into the source code.

You can look at here before the revert: echoix#202 or a combined 4 PRs ahead + coverage echoix#206

@echoix
Copy link
Member Author

echoix commented Aug 27, 2024

I suspect that the resourcewarnings are captured in a file that is used somewhere, like if a shell script redirected 2 & 1 together to a file and uses it as an input. That's why I think changing both sides isn't a good idea.

But anyways, this PRs doesn't touch the stds_export with old code, it was some fresh work from tonight, on another file.

@echoix echoix requested a review from wenzeslaus August 29, 2024 17:06
@echoix echoix merged commit 7df0e7f into OSGeo:main Aug 29, 2024
26 checks passed
@neteler neteler added this to the 8.5.0 milestone Aug 30, 2024
a0x8o pushed a commit to a0x8o/grass that referenced this pull request Sep 5, 2024
@echoix echoix deleted the stds_import-read-write-whole-file branch September 8, 2024 18:56
Mahesh1998 pushed a commit to Mahesh1998/grass that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants