Skip to content

First dirty commit of TTE codes #57

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

Merged
merged 4 commits into from
Mar 26, 2025
Merged

Conversation

margauxraguenel
Copy link
Contributor

@margauxraguenel margauxraguenel commented Feb 27, 2025

This PR closes Issue #56, first step of #54.

The idea is to share the codes developped by TTE (mainly @alexbenedicto, @untereiner and @mlemayTTE) to everyone.
This will be the basis for the new python packages architecture.

We plan to work on refactoring the repo in the next weeks, and need to include all the material of this PR for this.

@untereiner
Copy link

great @margauxraguenel !
I would suggest to maybe add a SPDX-License-Identifier: xxxx in the .py files for geos-posp, geos-trame replacing the xxx with the chosen license and also a SPDX-FileCopyrightText: xxx for the geos-trame package that is missing

Copy link

@untereiner untereiner left a comment

Choose a reason for hiding this comment

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

a very good "first dirty commit" :)

Copy link
Contributor

@alexbenedicto alexbenedicto left a comment

Choose a reason for hiding this comment

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

Great work and documentation

Copy link
Collaborator

@CusiniM CusiniM left a comment

Choose a reason for hiding this comment

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

It's a huge PR with a lot of files so it's a hard to review. In general it looks like a good start. However, I would be a bit careful about adding all these images to the repository. Please make sure that the repo size does not explode. I would maybe suggest rewriting history and using git lfs for some of these.

@MelReyCG
Copy link
Contributor

@CusiniM @margauxraguenel
Hi Matteo,
As merging in develop is done with a squashed commit & removal of the PR branch, would it suffice to revert the commit here, and re-commit them with proper git-lfs use?

@CusiniM
Copy link
Collaborator

CusiniM commented Mar 10, 2025

@CusiniM @margauxraguenel Hi Matteo, As merging in develop is done with a squashed commit & removal of the PR branch, would it suffice to revert the commit here, and re-commit them with proper git-lfs use?

Hmmm... branches can always be restored and I don't really know where that history gets stored. I am assuming that if those commits never make into develop the size of the history should not be affected but I am not 100% sure.

@untereiner
Copy link

untereiner commented Mar 10, 2025

Hello, you can use git lfs migrate on this branch. It will rewrite the history of this branch only. After a force push you will have a clean history for this branch before you merge it in develop

git lfs migrate import --include="*.png *.jpeg" 
git lfs ls-files # list LFS files
git push --force # force push branche to remote

@margauxraguenel margauxraguenel force-pushed the raguenel/feature/tte-code-sharing branch from 8d02b27 to 6edfa79 Compare March 12, 2025 17:05
@margauxraguenel
Copy link
Contributor Author

Hi @CusiniM :-)

We added the pictures into lfs and rebased the history.
I think it is ready to merge.

Thanks !

@CusiniM
Copy link
Collaborator

CusiniM commented Mar 14, 2025

Hi @CusiniM :-)

We added the pictures into lfs and rebased the history. I think it is ready to merge.

Thanks !

It seems to me that some jpg are still not commited with lfs. If you need them for the documentaiton, we should make sure that readthedocs works with lfs though.

@margauxraguenel
Copy link
Contributor Author

We have checked the pictures and it seems ok for us.
Can you point us towards the files that are not ok for you if some are remaining ?

These images are linked to the user_guide.md file, which is currently not included in the CI. The documentation is independant for now, and we plan to adress this in a future PR.

@untereiner
Copy link

It seems that all JPG files are listed as lfs stored files. Don't know why github is displaying them

➜  geosPythonPackages git:(raguenel/feature/tte-code-sharing) git lfs ls-files
0f62ab34ad * docs/.fonts/Carlito-Bold.ttf
380764b689 * docs/.fonts/Carlito-BoldItalic.ttf
718a066386 * docs/.fonts/Carlito-Italic.ttf
b4ff23ba37 * docs/.fonts/Carlito-Regular.ttf
291e4388a4 * docs/.fonts/Poppins-Black.ttf
a5e3e31e9d * docs/.fonts/Poppins-BlackItalic.ttf
7219547ee2 * docs/.fonts/Poppins-Bold.ttf
9d4d9f3c2c * docs/.fonts/Poppins-BoldItalic.ttf
94a215f88f * docs/.fonts/Poppins-ExtraBold.ttf
bba986e116 * docs/.fonts/Poppins-ExtraBoldItalic.ttf
60c4bb1b8f * docs/.fonts/Poppins-ExtraLight.ttf
05418f4d33 * docs/.fonts/Poppins-ExtraLightItalic.ttf
3225cec6a0 * docs/.fonts/Poppins-Italic.ttf
647f014d36 * docs/.fonts/Poppins-Light.ttf
6d00aa5531 * docs/.fonts/Poppins-LightItalic.ttf
8d909883de * docs/.fonts/Poppins-Medium.ttf
449f6bd907 * docs/.fonts/Poppins-MediumItalic.ttf
707fdc5c8b * docs/.fonts/Poppins-Regular.ttf
248c0244b3 * docs/.fonts/Poppins-SemiBold.ttf
74b31cbc29 * docs/.fonts/Poppins-SemiBoldItalic.ttf
95875f9ef0 * docs/.fonts/Poppins-Thin.ttf
043226b0e1 * docs/.fonts/Poppins-ThinItalic.ttf
b589061105 * docs/images/AttributeMapping.JPG
4d60f54b6f * docs/images/CreateConstantAttributePerRegionFilter.JPG
dda93e2bde * docs/images/ExtractMergeBlockFilter.JPG
e032536db1 * docs/images/GEOS_XML_Wells.JPG
953a6f9fc1 * docs/images/GEOS_XML_csv_export.JPG
467730e1da * docs/images/GEOS_XML_flowSolver.JPG
8ebb9c042b * docs/images/GEOS_XML_vtkOutput.JPG
fe626a9293 * docs/images/GEOS_filter_organisation.JPG
6986d4ed91 * docs/images/GEOS_log_Reader0.JPG
f08b02300d * docs/images/GEOS_log_Reader1.JPG
3d69c22cef * docs/images/GEOS_log_data_spreadsheet.JPG
3daa02723c * docs/images/GEOS_log_start.JPG
c0c8e0f2aa * docs/images/GEOS_log_time_step.JPG
632d323464 * docs/images/GeomechanicsAnalysis.JPG
d7dac6c43b * docs/images/GeomechanicsWorkflows.JPG
539736c21c * docs/images/MergeBlocksEnhanced.JPG
51cfd0672a * docs/images/MultiBlock_inspector.JPG
ad1239aa47 * docs/images/Python_View_Configurator.JPG
4b7a3ea8bd * docs/images/SurfaceGeomechanics.JPG
8a7b576cad * docs/images/TransferAttributes.JPG
d33def2d68 * docs/images/mohr_circle_plot.JPG
04c65ae286 * docs/images/pressure_vs_time.JPG
ca021c78d4 * docs/images/pvd_file.JPG
3bb5d122c1 * docs/images/stress_along_well.JPG

@alexbenedicto
Copy link
Contributor

After testing some features added like geos-trame and geos-xml-viewer, I encountered some portability issues that I am currently dealing with. I have a first fix for geos-trame and I am working on the one for geos-xml-viewer.

@CusiniM
Copy link
Collaborator

CusiniM commented Mar 17, 2025

After testing some features added like geos-trame and geos-xml-viewer, I encountered some portability issues that I am currently dealing with. I have a first fix for geos-trame and I am working on the one for geos-xml-viewer.

when you say portability issue is it because of the python version? or is it something else?
if so, I am in favor of simply defining a minimum version required. It could be something >= 3.10 (https://devguide.python.org/versions/) People should not use their system python anyways.

@alexbenedicto
Copy link
Contributor

@CusiniM Yes and I would even suggest going to python >= 3.9 because it respects the python built with GEOS (3.9.18).
There is also the way to define the pyproject.toml that is different than the one implemented by Chris in the repo. I encountered some issues for example with "poetry" and none when switching to "setuptools" in the build system:

[build-system]
requires = ["setuptools>=42", "wheel"]
build-backend = "setuptools.build_meta"

instead of

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

@alexbenedicto
Copy link
Contributor

After last commit, the CI and the build for each package are good. @CusiniM I think it can be merged

@herve-gross
Copy link

@CusiniM if you have a moment, please check this out

@CusiniM CusiniM merged commit c8663ab into main Mar 26, 2025
31 checks passed
@alexbenedicto alexbenedicto deleted the raguenel/feature/tte-code-sharing branch March 27, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants