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

Projection.initialize() method #40

Open
paulushub opened this issue Aug 15, 2019 · 6 comments
Open

Projection.initialize() method #40

paulushub opened this issue Aug 15, 2019 · 6 comments

Comments

@paulushub
Copy link

The initialization method defined in the base class Projection is not called in this case.
Clearly, classes that extend the base projection class are required to call this method to initialize the projection parameters - by my understanding of the codes.

However, many classes extending the Projection class (like LandsatProjection class) do not provide the default constructor and most likely the initialize() method is never called in such cases.
Are the users supposed to call the initialize() method? If not how about consider making it protected method?

@pomadchin
Copy link
Member

pomadchin commented Aug 15, 2019

The story is that lots of projections (i.e. LandsatProjection) require some love as well as the entire project code (actually we're an open source project, you can help us). We know that there are lot's of issues with the codebase.

In case of initialize, I'm pretty sure 1. it is not required in all the cases, and I don't see how did you figure out that it is required everywhere - all projections rely on different parameters. Mb only from the code hygiene point. 2. the LandsatProjection doesn't properly work.

@paulushub
Copy link
Author

@pomadchin I think the comments to the Projection.initialize() method says a lot about its uses.

    /**
     * Initialize the projection. This should be called after setting parameters and before 
     *  using the projection.
     * This is for performance reasons as initialization may be expensive.
     */
    public void initialize() {
        spherical = (e == 0.0);
        one_es = 1-es;
        rone_es = 1.0/one_es;
        totalScale = a * fromMetres;
        totalFalseEasting = falseEasting * fromMetres;
        totalFalseNorthing = falseNorthing * fromMetres;
    }

Looking at the codes, the projections that require further initialization override this method and in all cases call the base method.

The easiest way to manage such project is to file known issues as tasks, otherwise, it is not obvious when you download the sources. I have done that with my current project on GitHub
SharpVectors/issues

I am running into these issues, because I started porting it to .NET/C#, which gives me the opportunity to look at the sources file by file, and I am filing these issues to draw attention to them.

@pomadchin
Copy link
Member

@paulushub are you sure that all projections use
spherical, one_es, rone_es, totalScale, totalFalseEasting, totalFalseNorthing?

For instance Landsat uses none of these parameters => that is why the lack of this call won't break anything. Mb for the code hygiene purposes it makes sense to make code more readable though.

I'm pretty sure that all non standard projections don't use common parameters and it can explain the lack of init method and lack of overrided constructors.

I see, here is the pointer to tests with non working projections we have a bunch of tests that are failing due to some bad math. Hope it will help you.

@pomadchin
Copy link
Member

Also I appreciate a lot all the issues created, thank you!

@paulushub
Copy link
Author

@paulushub are you sure that all projections use

Possibly not, but since this is also a port of the C/C++ codes, it is safer to simply follow the pattern defined in the comment, rather than searching to find if a project/inverse needs that or not.

@pomadchin
Copy link
Member

I would like even to get rid of the init method tbh.

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

No branches or pull requests

2 participants