nialov / fractopo Goto Github PK

I think it could be nice to add a link to the docs about validation errors (https://fractopo.readthedocs.io/en/latest/validation/errors.html) directly in the repo readme in the section trace validation, such that the reader can directly get an impression what fractopo checks there.

Validation (Validation(...).run_validation()) fails with TypeError: within_bounds() takes 6 positional arguments but 7 were given if the tracemap shapefiles read by geopandas contain z coordinates.

I worked around this by removing the z coordinate prior to validation using this code:

from shapely.geometry import Polygon, MultiPolygon, LineString

class_lookup = dict(Polygon=Polygon, MultiPolygon=MultiPolygon, LineString=LineString)


def remove_z_coordinates(geometries):
    """
    :param geometries: Shapely geometry iterable
    :return: a list of Shapely geometries without z coordinates
    """
    return [shapely.wkb.loads(shapely.wkb.dumps(geometry, output_dimension=2)) for geometry in geometries]


trace_data.geometry = remove_z_coordinates(trace_data.geometry)

In case something similar is your preferred fix.

Thanks for developing a great tool!

Edit: Simpler solution from one of the comments here

Hi,

In the documentation the recommendation is to ignore warnings. I think this is potentially a bad idea as the warnings are usually there for a reason and it could have unintended consequences for other libraries that may be used in the same workflow. Would it be possible to filter the specific warnings which you deem at unncessary?

As far as I see it, when invoking fractopo tracevalidate PATH_TO_TRACES PATH_TO_AREA, the default is that the validated data is saved in a sub-directory of the input data (e.g. PATH_TO_TRACES)?

That may be intentional, but I was wondering if it is more intuitive to put that data into sub-directories of the current working dir per default?

I'm analysing a large scale fault tracemap (>500 m length trace). If I set the snap threshold to 10 m, I get the error: ValueError: Found endpoint in replace_endpoint dict..

Unfortunately the tracemap is confidential which I realise makes it very hard for you to debug, but I thought I'd share in case you could suggest the best approaches for me to debug?

Hi,

I'm just starting my review of the code for JOSS.

I'm using a google colab environment and have installed fractopo using pip install git+https://github.com/nialov/fractopo - so this should be the most recent version of the code.

I have tried to run your example https://fractopo.readthedocs.io/en/latest/notebooks/fractopo_network_1.html but I am getting a matplotlib error.

ValueError                                Traceback (most recent call last)
[<ipython-input-12-a892d99a2aba>](https://localhost:8080/#) in <cell line: 1>()
----> 1 fit, fig, ax = kb11_network.plot_trace_lengths()

5 frames
[/usr/local/lib/python3.9/dist-packages/matplotlib/axis.py](https://localhost:8080/#) in set_ticks(self, ticks, labels, minor, **kwargs)
   2070         """
   2071         if labels is None and kwargs:
-> 2072             raise ValueError('labels argument cannot be None when '
   2073                              'kwargs are passed')
   2074         result = self._set_tick_locations(ticks, minor=minor)

ValueError: labels argument cannot be None when kwargs are passed

I'm using matplotlib 3.7.1. and have a gist here if you want to try running it https://gist.github.com/lachlangrose/506df31fdd81019df4a7e1d5126cac40

At the end of the summary, it says

"
To analyze these datasets, common Geographic Information System (GIS) tools are typically used to perform geospatial operations and analysis of the data. However, these tools are not specialized to handle the specific requirements for geometric and topological consistency of the fracture trace data. Therefore, it is both convenient and beneficial for users to have access to a common workflow, provided by the newly developed fractopo package, for the data validation and analysis, rather than implementing a geospatial analysis separately each time for the data.
"

This is just a semantic thing, but I don't understand the causality between the first two sentences and the last one. The way I understand the first two, GIS tools are missing functionality for checking fracture trace data consistency. But the last sentence says that because of that, it's good to have access to a common workflow. Here, I would have rather expected something along the lines that fractopo seeks to close this gap. Also, I cannot relate the last argument - "rather than implementing a geospatial analysis separately each time for the data." - to the stuff said before. This means the analysis can be done in GIS tools but I have to implement it again for each data? (That is, the tools don't allow to define an analysis pipeline in which I can just swap the input data?)

I have the feeling that two points are intermingled here and it may be more clear if they were laid out separately:

1. fractopo provides missing functionality in typical GIS tools (for the validation part)
2. fractopo provides both validation & analysis in one package, usable outside of GIS tools (e.g. in scripts). This enables users to script their analysis pipeline once and send through different data as desired

Is this correct? In case I am getting it all wrong, my apologies in advance...

Hi Nikolas,

I wrote you a mail earlier today asking about the state of your tool. I downloaded your package to try it out.
In the "trace validation" section of the documentation there is the object "Validation" which I cannot import.

I get the error message:
ImportError: cannot import name 'Validation' from 'fractopo' (unknown location)

Have you renamed or changed this object in the meantime?

Thanks a lot in advance and best wishes,
Martin

Currently, the CLI invocation is fractopo tracevalidate TRACES AREA as far as I see it. While this is quite intuitive, in the very beginning I had to double check which one comes first. I also then tried swapping the two, and I seem to recall that this didn't lead to an error message that makes it directly obvious what went wrong. Therefore, I was wondering if tracevalidate should receive just the traces as a positional argument, and the area as one that is passed with a name, e.g. fractopo tracevalidate traces.geojson --area my_area.geojson.

From the README in the repo, clicking on a tutorial link works for me and brings me to the correct position in the full documentation. However, when scrolling through the start page of the full documentation, clicking on the links shown there bring me to some place in the full docs' main page, but not the actual tutorial.

Not sure if there is an easy fix for that, but maybe you want to take a look.

In the first notebook in the contour grids section, the variable network needs to be changed to kb11_network

From: openjournals/joss-reviews#5300

Line 10-13: I suggest revising this sentence to something like "Digitizing these fracture traces from observations (e.g., drone imaged outcrops) results in georeferenced two-dimensional fracture trace vector datasets (i.e., fracture networks), which offer..." to help make it clear what text the "e.g.," and "i.e.," apply to.

Line 35: Consider a small revision to reword "boundary to fractopo based on which a number of analysis results..." to something like "boundary to fractopo, from which a number of analysis results..."

Line 38: Is the "e.g." needed here? If these are all examples I suggest simply saying something like "Example results (Figure 2) include rose plots ... "

Line 47-48: A minor text suggestion "package managers as well as containing features absent..." could be revised to something like "package managers and contains features absent..."

Originally posted by @elbeejay in openjournals/joss-reviews#5300 (comment)

I was reading through the readme file and stumbled across a few (minor) things. I'll post them here in case you want to consider them:

• The readme says "As it is a Python library, the use of fractopo requires prior (Python) programming knowledge.". I have the feeling that the CLI may already be useful for people, so maybe I would rephrase or remove this sentence not to scare off some people.
• (this is actually not so minor) When I tried the instructions using a local installation with pip from the sources with pip install --editable .[dev], I got a warning that the extra dev is not defined. This actually causes a bunch of follow-up errors. Because if you then try to run poetry run pytest, one gets a long list of errors from missing modules... I think the pip install should either be fixed, or, the instructions removed, requiring users to rely on poetry
• In the analysis example (https://github.com/nialov/fractopo#geometric-and-topological-trace-network-analysis), it would be good if the comment above the snap_threshold stated the unit in which this is given?
• The development section (https://github.com/nialov/fractopo#geometric-and-topological-trace-network-analysis) starts with "Breaking changes are possible and expected." This doesn't sound super inviting for people that want to use the package :). Since you have a changelog, maybe one can add that any breaking changes will be properly documented therein?

Regarding the validation error docs: are these errors physically motivated (i.e. to detect non-physical topologies) or are they there for some other technical reason? I guess some are technical, some are physically-motivated. Do you think it makes sense to say a word or two on that in the respective error sections? Some actually contain very useful background information, for instance, the MultiLineString error.

For the sharp-corner-validator, the docs say

"... is completely open to interpretation and therefore the resulting errors are subjective."

Wouldn't a similar argument also hold for other errors? For instance, under- or overlapping snap? For instance, the underlapping-snap-docs say

"Underlapping error can occur when a trace ends very close to another trace but not near enough. The abutting might not be registered as a Y-node."

In this context, "not near enough" means it's further away than the given snap_threshold? If this is correct, then I guess the threshold would be the subjectivity? But on the other hand, I then wonder why it should be an error? I am probably missing something...

Originally posted by @dglaeser in #31 (comment)

fractopo tracevalidate --help shows for the allow-fix option

--allow-fix,--fix                                             Allow the direct modification of trace file to fix errors. [default: True]   

Does this really modify the file (i.e. resaving it to disk)? If so, I am unsure if True should be the default. Modification of the input file should be explicitly requested I would say. Or does this rather mean that the errors are fixed and then the fixed version of the traces are saved into output? if the latter is the case, maybe the docstring for this CLI parameter could be adjusted a bit?