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

feat: Python interfaces to geos from Makutu repo #44

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

av-novikov
Copy link
Contributor

@av-novikov av-novikov commented Oct 30, 2024

This PR merges some Python interfaces to GEOS from Makutu repository. These interfaces are coming as a part of GEOS: PR 3391 and required to simplify interactions with GEOS from Python.

  • place interfaces to pygeos-tools

@untereiner
Copy link

as a minor comment: change # Copyright (c) 2018-2024 Total, S.A to # Copyright (c) 2018-2024 TotalEnergies as (partially) done in the GEOS repo (see PR #1562)

Copy link

@Bubusch Bubusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @av-novikov, looks good.
@sframba @Victor-M-Gomes maybe at some point we can move all pygeos-tools/utilities up under pygeos-tools/ and make the package a bit more flat. We might also have to set some constraints on dependencies versions when installing on ppc architecture.

@alexbenedicto
Copy link
Contributor

@av-novikov Hello! I am currently working in the geos-mesh repository, which includes Python packages for pre-processing VTK grids, specifically through a tool called mesh_doctor.

I've noticed that some of the features you are adding might overlap with existing functionalities in mesh_doctor. To avoid redundancy, I suggest we collaborate to identify and merge these features effectively. This way, we can enhance mesh_doctor without duplicating efforts across two repositories. I'm happy to assist with this process.

@cssherman, what are your thoughts on this approach?

@av-novikov
Copy link
Contributor Author

av-novikov commented Nov 15, 2024

as a minor comment: change # Copyright (c) 2018-2024 Total, S.A to # Copyright (c) 2018-2024 TotalEnergies as (partially) done in the GEOS repo (see PR #1562)

fixed, thanks @untereiner !

@av-novikov Hello! I am currently working in the geos-mesh repository, which includes Python packages for pre-processing VTK grids, specifically through a tool called mesh_doctor.

I've noticed that some of the features you are adding might overlap with existing functionalities in mesh_doctor. To avoid redundancy, I suggest we collaborate to identify and merge these features effectively. This way, we can enhance mesh_doctor without duplicating efforts across two repositories. I'm happy to assist with this process.

Dear @alexbenedicto, thanks for looking inside! I can imagine that some of this functionality my overlap with existing and I am happy to contribute to the further development of the Python side of GEOS. However, this particular functionality is taken from Makutu-team repo as is. Thus, it is better to coordinate further its modification with Makutu developers, e.g. with @sframba.

@av-novikov av-novikov merged commit 83e92b6 into main Nov 15, 2024
26 of 31 checks passed
@CusiniM
Copy link
Collaborator

CusiniM commented Nov 20, 2024

I am reverting this merge. The repository had no restrictions in place but PRs this large should not be merged with a single approving reviewer, especially considering that there was a comment by @alexbenedicto pointing out that he would have liked to discuss how to proceed considering the amount of duplicated capabilities.

Please discuss with @alexbenedicto about how to proceed before remerging this.

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

Successfully merging this pull request may close these issues.

5 participants