brycefrank / pyfor

Tools for analyzing aerial point clouds of forest data.
MIT License
94 stars 19 forks source link

Can't write to file when acting as class object? #40

Closed bw4sz closed 5 years ago

bw4sz commented 5 years ago

I ran into a strange error that might be demonstrative of something larger. Forgive the lack of reproducibility.

Consider this pyfor object. It was just clipped from a larger plot.

generator.clipped_las
<pyfor.cloud.Cloud object at 0x14e07c438>

It looks normal

generator.clipped_las.data.points.head()
     index           x            y   ...    bins_x  bins_y  bins_z
0  2336576  315006.189  4091000.072   ...         6      37       0
1  2336577  315005.268  4091000.146   ...         5      37       0
2  2336578  315004.345  4091000.222   ...         4      37       0
3  2336579  315003.436  4091000.298   ...         3      37       0
4  2336580  315002.527  4091000.370   ...         2      37       0

and I can see the method

generator.clipped_las.write
<bound method Cloud.write of <pyfor.cloud.Cloud object at 0x14e07c438>>

but then

generator.clipped_las.write(path="/Users/Ben/Desktop/test.laz")
Traceback (most recent call last):
  Debug Probe, prompt 99, line 1
  File "/Users/ben/miniconda3/envs/DeepLidar_dask/lib/python3.6/site-packages/pyfor/cloud.py", line 350, in write
    self.data.write(path)
builtins.AttributeError: 'CloudData' object has no attribute 'write'

Nothing is wrong with my writer

pc=pyfor.cloud.Cloud("/Users/ben/Documents/DeepLidar/data/TEAK/TEAK_044.laz")
pc.write("/Users/Ben/Desktop/test2.laz")

Nor is it the clipping. Here is the same behavior with the original tile.

generator.lidar_tile.write(path="/Users/Ben/Desktop/fulltest.laz")
Traceback (most recent call last):
  Debug Probe, prompt 107, line 1
  File "/Users/ben/miniconda3/envs/DeepLidar_dask/lib/python3.6/site-packages/pyfor/cloud.py", line 350, in write
    self.data.write(path)
builtins.AttributeError: 'CloudData' object has no attribute 'write'

some use of super class methods in pyfor? Something is weird here. Happy to dig.

brycefrank commented 5 years ago

Ben,

Yes, I have been running into this as well. The issue started popping up when LASData and PLYData become children of CloudData.

If you dig into Cloud.clip you will see, probably, that it returns a CloudData object, which, as the error indicates, does not have a .write method.

The challenge is to create the correct *Data object after a clip in an elegant way. This could be done with some conditionals somewhere, but I have not figured out a good place where. Open to ideas. (forgive me for being brief, a bit busy today)

bw4sz commented 5 years ago

Okay, sort of starting to understand, here is the example i'm playing with.

import pyfor
import os
import geopandas as gpd

#Read in data
test_laz = pyfor.cloud.Cloud("/Users/ben/Documents/pyfor/pyfortest/data/test.laz")
test_shp = os.path.join('/Users/ben/Documents/pyfor/pyfortest/data/', 'clip.shp')

test_laz.write("/Users/ben/Desktop/firstsave.laz")

poly = gpd.read_file(test_shp)['geometry'][0]
clipped=test_laz.clip(poly)

clipped.data.points.head()

test_laz.write("/Users/ben/Desktop/secondsave.laz")
clipped.write("/Users/ben/Desktop/clippedsave.laz")

In my case, its weird, even the original tile has this problem. This issue might go all the way up to normalize as well. Something about the in_place replacement.

brycefrank commented 5 years ago

Ben, I should have a chunk of time tomorrow to run your above example. I can do it first thing in the morning (my morning that is!).

bw4sz commented 5 years ago

no rush, i've redirected to other parts of the code base.

brycefrank commented 5 years ago

Ben,

It turns out I already have the nasty conditional I was envisioning already written in Cloud, so I just leveraged that to handle arbitrary *Data objects. It isn't pretty but it should work. Your above script ran fine after the fix.

My tests passed on local, and I am 90% sure they will pass travis as well, I will push the change here in a minute and add a post documenting the new lines for my own reference.

brycefrank commented 5 years ago

The following structure was fleshed out in Cloud

https://github.com/brycefrank/pyfor/blob/b2f0be671257153366ee7d06ff8dec9704d05fee/pyfor/cloud.py#L114-L122

It is messy, but it at least builds on what was already there.

Using the issue of clipping as an example: clipping returns a new Cloud object, which itself has a new CloudData object as its .data attribute. A few updates ago I split CloudData into a parent/child structure, with LASData and PLYData as children, however .clip was still only instantiating CloudData objects, which do not have the particular write method given the desired file type.

The above fix checks what type of data.header exists in the new object and converts the .data attribute to the appropriate type.

bw4sz commented 5 years ago

Pulled and confirmed.

bw4sz commented 5 years ago

There is still something to this. I'll try to create something reproducible tomorrow. Maybe during normalization? I couple different things happen so i'll need to check.

MacBook-Pro:pyfor ben$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
nothing to commit, working tree clean
MacBook-Pro:pyfor ben$ git fetch
MacBook-Pro:pyfor ben$ git merge upstream/master
Already up-to-date.
MacBook-Pro:pyfor ben$ conda activate DeepLidar_dask
(DeepLidar_dask) MacBook-Pro:pyfor ben$ pip install .
Processing /Users/ben/Documents/pyfor
Building wheels for collected packages: pyfor
  Running setup.py bdist_wheel for pyfor ... done
  Stored in directory: /private/var/folders/kj/36jkrhvx2w9f9fn0k7glzf5h0000gn/T/pip-ephem-wheel-cache-huwcjvyx/wheels/26/19/01/5918682f9f838ac742c3c0690ad9c3b3a3bc2972b067a539ae
Successfully built pyfor
tensorflow 1.10.1 has requirement numpy<=1.14.5,>=1.13.3, but you'll have numpy 1.15.3 which is incompatible.
tensorflow 1.10.1 has requirement setuptools<=39.1.0, but you'll have setuptools 40.2.0 which is incompatible.
Installing collected packages: pyfor
  Found existing installation: pyfor 0.3.2
    Uninstalling pyfor-0.3.2:
      Successfully uninstalled pyfor-0.3.2
Successfully installed pyfor-0.3.2
You are using pip version 10.0.1, however version 19.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
def load_lidar(laz_path):
    """
    Load lidar tile from file based on path name
    param: A string of the file path of the lidar tile
    return: A pyfor point cloud
    """

    try:
        pc=pyfor.cloud.Cloud(laz_path)
        pc.extension=".las"

    except FileNotFoundError:
        print("Failed loading path: %s" %(laz_path))

    #normalize and filter
    pc.normalize(1)    

    #Quick filter for unreasonable points.
    pc.filter(min = -1, max = pc.data.points.z.quantile(0.995), dim = "z")    

    #Check dim
    assert (not pc.data.points.shape[0] == 0), "Lidar tile is empty!"

    return pc
pc = load_lidar("/Users/Ben/Documents/DeepLidar/data/TEAK/TEAK_043.laz")
pc.write("/Users/Ben/Desktop/test.laz")
Traceback (most recent call last):
  Python Shell, prompt 64, line 1
  File "/Users/ben/miniconda3/envs/DeepLidar_dask/lib/python3.6/site-packages/pyfor/cloud.py", line 350, in write
builtins.AttributeError: 'CloudData' object has no attribute 'write'
brycefrank commented 5 years ago

I will look into this tomorrow hopefully. I am very slowly chewing through this Travis bug still (it requires rebuilding the environment at each iteration, so it is a bit of a bear).

bw4sz commented 5 years ago

me too. Its a mystery to me, because obviously for the other issue, i wrote a file. So it must be a specific sequence. Can you confirm that

pyfor.__version__
'0.3'

is the desired behavior, even though we are on 0.3.2, just trying to sort out if i've got some anaconda problem that i don't expect.

bw4sz commented 5 years ago

https://github.com/brycefrank/pyfor/blob/2108ac9008243842d9c90f41a5b9409a97fd22db/pyfor/__init__.py#L3

bw4sz commented 5 years ago

confirming that this example still works

import pyfor
import os
import geopandas as gpd

#Read in data
test_laz = pyfor.cloud.Cloud("/Users/ben/Documents/pyfor/pyfortest/data/test.laz")
test_shp = os.path.join('/Users/ben/Documents/pyfor/pyfortest/data/', 'clip.shp')

test_laz.write("/Users/ben/Desktop/firstsave.laz")

poly = gpd.read_file(test_shp)['geometry'][0]
clipped=test_laz.clip(poly)

clipped.data.points.head()

test_laz.write("/Users/ben/Desktop/secondsave.laz")
clipped.write("/Users/ben/Desktop/clippedsave.laz")

so, the above thread still is valid.

brycefrank commented 5 years ago

Ah, it seems I missed that line.

0.3.2 is the correct version. It may be more robust if you use git show given my lazy versioning ;)

I will look into #43 to alleviate this

brycefrank commented 5 years ago

Ben,

Keeping you posted on some thoughts here:

After this is fixed and some other minor issues I will wrap up a proper 0.3.3 version. I think it is finally time to properly tackle handling .laz files with pylas (a package different from laspy that handles laz a bit more efficiently).

That said, I imagine this will restructure a lot of the file I/O as it is currently written. I will search for work-able hotfix for this bug, but it will be a temporary solution with this larger goal in mind.

brycefrank commented 5 years ago

Ben,

I tracked down an easy fix regarding the filtering issue (https://github.com/brycefrank/pyfor/issues/40#issuecomment-459564876). It should be live on 0.3.3 within the hour. Pending some tests, I will merge 0.3.3 to master today as well.