-
Notifications
You must be signed in to change notification settings - Fork 0
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
Aot testing #33
base: main
Are you sure you want to change the base?
Aot testing #33
Conversation
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> | ||
</None> | ||
|
||
<Content Include="C:\Users\HEC\Downloads\GDAL\GDAL\bin64\*.dll"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These guys will always fail because we don't have the GDAL Build. We're going to need to set up some system to check if GDAL exists. Check if it's the right GDAL (optionally) , and if we don't, download it. This is a problem FDA has to solve too. We'll solve it together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's our GDAL build. Look into Build events to trigger this: https://learn.microsoft.com/en-us/visualstudio/ide/how-to-specify-build-events-csharp?view=vs-2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tell me more about this file. I'm not familiar with it. Why does it exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this folder got added automatically when I checked the "allow native debugging" option in Visual Studio. Breakpoints in my native methods weren't getting hit, so I had to change that option. The JSON in the file contains an entry specifying that native debugging is allowed.
Consequences/ExternalMethods/NSI.cs
Outdated
public class NSI | ||
{ | ||
[UnmanagedCallersOnly(EntryPoint = "ReadNSI")] | ||
public static int Read() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not that useful as an external method because the caller can't do anything with the data. Might make more sense to do a "WriteNSItoShapefile" with x and y params to specify the bounding box, and a string file path. for the output file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left this in as a test method and forgot to actually make it useful. For now I'll have a method that just reads from the NSI in case a user just has the Consequences project, and then in GeoConsequences I'll have a method that reads from the NSI and writes to shapefile
Geospatial/OGR/SpatialWriter.cs
Outdated
_headersWritten = true; | ||
} | ||
|
||
using var feature = new Feature(_layer.GetLayerDefn()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invert the naming. Put type on the left whenever possible. Same for the rest of this method.
|
||
using var feature = new Feature(_layer.GetLayerDefn()); | ||
using var geometry = new Geometry(wkbGeometryType.wkbPoint); | ||
double x = (double)res.Fetch(_xField).ResultValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know x will be a double? Might want to check for that and throw an exception. Document somewhere up the chain that we demand it to be as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fetch.ResultValue returns an object so if that can't be cast to a double, it should already throw an exception. I'll make a note of that somewhere
} | ||
|
||
public void Dispose() | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this guy doing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class has to implement IDisposable, so I had to include this in order to do so. I don't know what the method should do, though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this guy should be committing any transactions that have not been committed and releasing any pointers to any gdal c objects - so that the memory can be garbage collected and so that the data in its entirety gets flushed to disk.
I lied to you about the file conflicts of merging that other pull request. I apologize. Will need to do the merge reconciliation., |
No description provided.