Skip to content

merge save-aps into v2beta #75

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

Open
wants to merge 77 commits into
base: v2beta
Choose a base branch
from

Conversation

mahmud1
Copy link
Member

@mahmud1 mahmud1 commented Mar 31, 2025

No description provided.

mahmud1 and others added 30 commits August 2, 2024 13:16
# Conflicts:
#	docs/demo/demo_masjed_dam_detailed_guide.rst
#	docs/processing.rst
#	sarvey/config.py
#	sarvey/densification.py
#	sarvey/processing.py
#	tests/test_processing.py
#	tests/testdata/config_test.json
# Conflicts:
#	docs/processing.rst
#	sarvey/filtering.py
#	sarvey/processing.py
mahmud1 and others added 28 commits February 19, 2025 14:39
merge upstream into main
merge main into update-runner2
- update runner with pip instllation
- update docs
- move environment.yml to root
update dev installation in docs
# Conflicts:
#	HISTORY.rst
#	sarvey/preparation.py
#	sarvey/processing.py
#	sarvey/utils.py
@mahmud1 mahmud1 requested a review from Copilot March 31, 2025 15:49
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR merges the "save-aps" branch into v2beta and updates the code to use map coordinates instead of UTM coordinates. Key changes include:

  • Updating dependency constraints and adding an extra requirement ("gdal") in setup.py.
  • Replacing UTM coordinate variables and function parameter names with their map coordinate counterparts across multiple modules.
  • Adjustments to APS estimation, densification logic, configuration validations, and CI pipeline modifications.

Reviewed Changes

Copilot reviewed 19 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
setup.py Updated dependency constraints and added an extra dependency.
sarvey/utils.py Switched coordinate calculations from UTM to map coordinates.
sarvey/triangulation.py Updated parameter names from coord_utmxy to coord_map_xy.
sarvey/sarvey_plot.py Updated function calls to use map coordinates.
sarvey/processing.py Replaced UTM references with map coordinates and updated APS handling.
sarvey/export.py Updated coordinate transformations and CRS settings.
sarvey/objects.py Renamed and reworked coordinate class from CoordinatesUTM to CoordinatesMap.
sarvey/filtering.py Modified functions for atmospheric phase screen estimation using map coordinates.
sarvey/densification.py Updated densification logic and second-order points selection based on map coordinates.
sarvey/config.py Adjusted configuration validations and thresholds for densification.
.github/workflows/ci.yml Changed environment variable settings and CI environment activation.
Files not reviewed (6)
  • HISTORY.rst: Language not supported
  • README.rst: Language not supported
  • docs/demo/demo_masjed_dam_detailed_guide.rst: Language not supported
  • docs/demo/demo_masjed_dam_fast_track.rst: Language not supported
  • docs/installation.rst: Language not supported
  • docs/processing.rst: Language not supported
Comments suppressed due to low confidence (1)

.github/workflows/ci.yml:20

  • The environment variable changes for SKIP_DEPLOY and SKIP_INSTALL may affect the CI pipeline behavior. Please verify that these updated values match the intended deployment and installation requirements.
SKIP_DEPLOY: true
SKIP_INSTALL: false

Comment on lines +502 to +503
interp_method=aps_params_obj.params_obj.model_name)

Copy link
Preview

Copilot AI Mar 31, 2025

Choose a reason for hiding this comment

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

The code attempts to reference 'params_obj.model_name' on 'aps_params_obj', but the ApsParameters class defines 'model_name' directly. It likely should be updated to use 'aps_params_obj.model_name' to avoid an AttributeError.

Suggested change
interp_method=aps_params_obj.params_obj.model_name)
interp_method=aps_params_obj.model_name)

Copilot uses AI. Check for mistakes.

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.

2 participants