Get review from Copilot by meeqn · Pull Request #33 · meeqn/seacharts

meeqn · 2025-11-03T21:55:34Z

No description provided.

Changes for autosize

…milarly to fgdb

S-57 base functionality implementation

…dle northern and southern projection

updated configuration for different utm zones, display should now han…

bugfix

…charts_s57_WIP into konrad_branch

shallow waters are not underlapping whole map now

loading shapefiles simplified

Get records in layer

Utility updates

…conda (#21)

* Update README.md - features * Update README.md * Update README.md - usage * Update README.md - image * image added * image * image * Update README.md * Update README.md --------- Co-authored-by: miqn <109998643+meeqn@users.noreply.github.com>

* added some description for users running SC for the first time * upgraded test file and parametrized coastline display

* added some description for users running SC for the first time * upgraded test file and parametrized coastline display * seacharts 4 setup tips added

* added some description for users running SC for the first time * upgraded test file and parametrized coastline display * seacharts 4 setup tips added * Generated descriptions for most of the code and few refactors

* add weather utility function * Update config.yaml * Update config.yaml * Update config.yaml

* minor fixes regarding setup and documentation * simplified testing files

* Refactor configuration schema and core classes for improved clarity and functionality. Use CoordTuple for XY paired data in Extent class - it notably improves readability and thus maintainability. In the future these types of coordinates and bounding boxes could be replaced with similar, more readable containers. Changelog: - Updated comments in config_schema.yaml for better understanding of parameters. - Enhanced Config class to support optional config_path parameter. - Introduced CoordTuple class in extent.py for better handling of 2D coordinates. - Modified methods in extent.py to utilize CoordTuple for size, origin, and center. - Adjusted ENC class to return coordinates as tuples from extent. - Cleaned up imports and removed unused code in various files. - Improved type hints across multiple classes for better code readability. * Enhance directory structure creation and improve error handling for layer retrieval * Remove unused autosize option from config schema

Copilot

Pull Request Overview

This PR introduces SeaCharts version 4.0 with significant enhancements including S-57 maritime map format support, weather data visualization, and improved API functionality. The changes add support for parsing and displaying S-57 Electronic Navigational Charts alongside existing FGDB support, integrate weather data overlays with time-based controls, and provide new coordinate query methods.

Key Changes

S-57 Format Support: New parser (S57Parser) for IHO S-57 maritime chart format with layer extraction capabilities
Weather Visualization: Integration with PyThor weather service for displaying wind, waves, currents, and tides with interactive time slider controls
Enhanced API: Added methods for querying depth at coordinates, checking layer membership, and retrieving layer parameters

Reviewed Changes

Copilot reviewed 38 out of 42 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
user_doc/seacharts_user_doc.md	Comprehensive user documentation for v4.0 features
tests/test_seacharts_4_0.py	Test file demonstrating new S-57 and coordinate query functionality
tests/config_sc4.yaml	Configuration for S-57 map testing
seacharts/core/parserS57.py	New S-57 format parser implementation
seacharts/environment/weather.py	Weather data fetching and parsing logic
seacharts/display/display.py	Added weather visualization and control panel features
seacharts/core/extent.py	Enhanced coordinate transformation with WGS84 and UTM support
seacharts/layers/layer.py	Added parameter extraction at coordinates
seacharts/enc.py	New coordinate query methods for depth and layer membership

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-03T21:59:07Z

tests/test_seacharts_4_0.py

+import sys, os
+from shapely.geometry import MultiPolygon, Polygon
+# file made to showcase functionalities added in seacharts 4.0 integrated within old seacharts functionalities
+# for 


Incomplete comment on line 4 - appears to be a fragment that should be completed or removed.

Copilot · 2025-11-03T21:59:07Z

seacharts/display/display.py

+            if (bbox[i] != environment.scope.extent.bbox[i]):
+                changed.append(i)
+        if len(changed)>0:
+            print(Fore.RED + f"WARNING: Bouding box for display has exceeded the limit of CRS axes and therefore been scaled down. Watch out for potentially cropped chart display!" + Fore.RESET)


Corrected spelling of 'Bouding' to 'Bounding'.

Suggested change

print(Fore.RED + f"WARNING: Bouding box for display has exceeded the limit of CRS axes and therefore been scaled down. Watch out for potentially cropped chart display!" + Fore.RESET)

print(Fore.RED + f"WARNING: Bounding box for display has exceeded the limit of CRS axes and therefore been scaled down. Watch out for potentially cropped chart display!" + Fore.RESET)

Copilot · 2025-11-03T21:59:07Z

seacharts/display/display.py

+        """
+        Sets bounding box for the display taking projection's (crs's) x and y limits for display into account.
+        Making sure that bbox doesn't exceed limits prevents crashes. When such limit is exceeded, an appropriate message is displayed
+        to inform user about possibility of unexpeced display bound crop


Corrected spelling of 'unexpeced' to 'unexpected'.

Suggested change

to inform user about possibility of unexpeced display bound crop

to inform user about possibility of unexpected display bound crop

Copilot · 2025-11-03T21:59:08Z

seacharts/display/display.py

+
    def start(self) -> None:
-        """
+        self.started__ = """


Line 88 contains a syntax error: the docstring is incorrectly assigned to a variable self.started__ instead of being a proper docstring. This should be formatted as a standard docstring immediately after the function definition.

Suggested change

self.started__ = """

"""

Copilot · 2025-11-03T21:59:08Z

seacharts/display/display.py

+        if "wind_speed" and "wind_direction" in radio_labels:
+            radio_labels.append("wind")
+        if "wave_height" and "wave_direction" in radio_labels:
+            radio_labels.append("wave")
+        if "sea_current_speed" and "sea_current_direction" in radio_labels:


The boolean logic is incorrect. The expression if \"wind_speed\" and \"wind_direction\" in radio_labels will always evaluate the first string as truthy and only check if the second is in the list. It should be if \"wind_speed\" in radio_labels and \"wind_direction\" in radio_labels. The same issue exists on lines 748 and 750.

Suggested change

if "wind_speed" and "wind_direction" in radio_labels:

radio_labels.append("wind")

if "wave_height" and "wave_direction" in radio_labels:

radio_labels.append("wave")

if "sea_current_speed" and "sea_current_direction" in radio_labels:

if "wind_speed" in radio_labels and "wind_direction" in radio_labels:

radio_labels.append("wind")

if "wave_height" in radio_labels and "wave_direction" in radio_labels:

radio_labels.append("wave")

if "sea_current_speed" in radio_labels and "sea_current_direction" in radio_labels:

Copilot · 2025-11-03T21:59:10Z

seacharts/display/features.py

+        Adds an overlay geometry to the display.
+
+        :param geometry: The geometry to overlay.
+        :param color_name: The name of the color for the overlay.
+        :param fill: Whether the overlay should be filled.
+        :param linewidth: The width of the overlay line.
+        :param linestyle: The style of the overlay line.
+        :param alpha: The transparency level of the overlay.


The docstring parameters don't match the function signature. The function add_polygon has parameters shape, color, interiors, fill, linewidth, linestyle, and alpha, but the docstring references geometry and color_name instead.

Suggested change

Adds an overlay geometry to the display.

:param geometry: The geometry to overlay.

:param color_name: The name of the color for the overlay.

:param fill: Whether the overlay should be filled.

:param linewidth: The width of the overlay line.

:param linestyle: The style of the overlay line.

:param alpha: The transparency level of the overlay.

Adds a polygon overlay to the display.

:param shape: The exterior coordinates of the polygon, or a collection of polygons.

:param color: The color to use for the polygon.

:param interiors: A list of interior coordinates (holes) for the polygon.

:param fill: Whether the polygon should be filled.

:param linewidth: The width of the polygon outline.

:param linestyle: The style of the polygon outline.

:param alpha: The transparency level of the polygon.

Copilot · 2025-11-03T21:59:10Z

seacharts/display/display.py

+        draw_default = data is None
+        for i in range(len(direction_data)):
+            for j in range(len(direction_data[i])):
+                x = direction_data[i][j]


Variable x is not used.

Suggested change

x = direction_data[i][j]

Copilot · 2025-11-03T21:59:10Z

seacharts/core/parserS57.py

+        super().__init__(bounding_box, path_strings)
+        self.epsg = epsg
+
+    def get_source_root_name(self) -> str:


Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.

Copilot · 2025-11-03T21:59:11Z

seacharts/enc.py

+import _warnings
 from pathlib import Path
-
+from shapely.geometry import Point, Polygon


Import of 'Polygon' is not used.

Suggested change

from shapely.geometry import Point, Polygon

from shapely.geometry import Point

Copilot · 2025-11-03T21:59:11Z

seacharts/core/parserFGDB.py

+
+import fiona
+
+from seacharts.core import DataParser, paths


Import of 'paths' is not used.

Suggested change

from seacharts.core import DataParser, paths

from seacharts.core import DataParser

* Fixed some issues based on Copilot-generated review. - Added a check in `build_directory_structure` to raise a ValueError if the source root name is None. - Updated `get_source_root_name` method signatures in `DataParser`, `FGDBParser`, and `S57Parser` to indicate they can return None. - Modified return values in `FGDBParser` and `S57Parser` to return None when no valid data is found. - Improved docstrings for clarity on return types and conditions. * Fix the false positive scenarios in control panel creation

* Remove unused 'override' method * Change config to default to darkmode * Remove redundant markdown, update main README file

meeqn and others added 30 commits February 23, 2024 18:34

Initial changes, started work on format separation

5fbd7c4

Further changes regarding early parsing

d316396

3.03. changes - coordinates

e8b5c22

Changes for autosize

3582a85

Merge branch 'Natalia_Branch' into miqn_branch

334d318

Merge pull request #1 from meeqn/miqn_branch

d7f379c

Changes for autosize

3.03. changes - coordinates

8948ff8

3.03. changes - coordinates

46d1995

3.03. changes - parser S-57

fecb285

13.03. changes - zone

a46a4dc

Delete .idea directory

cac154d

config new

65e7ad6

Merge remote-tracking branch 'origin/Natalia_Branch' into Natalia_Branch

bca170f

lat long to UTM

6a31161

changed zone (one per map)

9ca1875

pre pull

6181aaf

pre pull

1802f74

Merge remote-tracking branch 'origin/Natalia_Branch' into miqn

d5fbf2f

working early S-57 maps conversion + display for requested depths, si…

18d2acc

…milarly to fgdb

Merge pull request #2 from meeqn/miqn_branch

0086640

S-57 base functionality implementation

updated configuration for different utm zones, display should now han…

f4a3896

…dle northern and southern projection

Merge pull request #3 from meeqn/miqn

bba05eb

updated configuration for different utm zones, display should now han…

fixed coordinate offsets

52e73e1

Merge pull request #4 from meeqn/miqn

00ad4db

bugfix

weather download implementation

eb5eea3

minor bug fix

df81fe1

Update weatherManager.py

532c971

weather data into layers

9489804

weather data into layers

4679eee

Merge branch 'konrad_branch' of https://github.com/SanityRemnants/sea…

67fac0f

…charts_s57_WIP into konrad_branch

meeqn and others added 25 commits September 30, 2024 15:23

path fix and label for slider deleted

6c1d907

Merge branch 'main' of https://github.com/meeqn/seacharts_s57

db45821

shallow waters are not underlapping whole map now

ce727dd

Merge pull request #16 from meeqn/reduce_depth_overlapping

686ebaf

shallow waters are not underlapping whole map now

loading shapefiles simplified

a15aca8

Merge pull request #17 from meeqn/reduce_depth_overlapping

7c6408f

loading shapefiles simplified

get records

28e4a73

get records

15da5ac

Merge pull request #18 from meeqn/get_arguments

bd9547e

Get records in layer

add warning for projection limit exceed

01ca537

.

63105cf

Merge pull request #19 from meeqn/utility_updates

8681063

Utility updates

import fix, comments clarified in config, config file for setting up …

3983e82

…conda (#21)

added some description for users running SC for the first time (#22)

117349f

Readme update (#20)

293dbb9

* Update README.md - features * Update README.md * Update README.md - usage * Update README.md - image * image added * image * image * Update README.md * Update README.md --------- Co-authored-by: miqn <109998643+meeqn@users.noreply.github.com>

Display tweaks (#23)

e6b38a0

* added some description for users running SC for the first time * upgraded test file and parametrized coastline display

Documentation update (#24)

a842b48

* added some description for users running SC for the first time * upgraded test file and parametrized coastline display * seacharts 4 setup tips added

Documentation update (#26)

8df73e7

* added some description for users running SC for the first time * upgraded test file and parametrized coastline display * seacharts 4 setup tips added * Generated descriptions for most of the code and few refactors

fixes

cf237a3

pushing user_doc to remote (#27)

330a949

Added get_value for weather data (#25)

abc2b25

* add weather utility function * Update config.yaml * Update config.yaml * Update config.yaml

Minor fixes & tests update (#29)

5753843

* minor fixes regarding setup and documentation * simplified testing files

getting parameters from layer (#30)

87f1426

removed the min depth constraint from schema (#31)

06c65b4

meeqn requested a review from Copilot November 3, 2025 21:55

Copilot AI reviewed Nov 3, 2025

View reviewed changes

meeqn added 3 commits November 27, 2025 19:09

Commit premerge suggestions (#35)

0ef9751

* Remove unused 'override' method * Change config to default to darkmode * Remove redundant markdown, update main README file

Add Table of Contents in README.md for improved navigation (#36)

080ff20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get review from Copilot#33

Get review from Copilot#33
meeqn wants to merge 113 commits intopr_review_testfrom
main

meeqn commented Nov 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	print(Fore.RED + f"WARNING: Bouding box for display has exceeded the limit of CRS axes and therefore been scaled down. Watch out for potentially cropped chart display!" + Fore.RESET)
	print(Fore.RED + f"WARNING: Bounding box for display has exceeded the limit of CRS axes and therefore been scaled down. Watch out for potentially cropped chart display!" + Fore.RESET)

	to inform user about possibility of unexpeced display bound crop
	to inform user about possibility of unexpected display bound crop

-        Adds an overlay geometry to the display.
-        :param geometry: The geometry to overlay.
-        :param color_name: The name of the color for the overlay.
-        :param fill: Whether the overlay should be filled.
-        :param linewidth: The width of the overlay line.
-        :param linestyle: The style of the overlay line.
-        :param alpha: The transparency level of the overlay.
+        Adds a polygon overlay to the display.
+        :param shape: The exterior coordinates of the polygon, or a collection of polygons.
+        :param color: The color to use for the polygon.
+        :param interiors: A list of interior coordinates (holes) for the polygon.
+        :param fill: Whether the polygon should be filled.
+        :param linewidth: The width of the polygon outline.
+        :param linestyle: The style of the polygon outline.
+        :param alpha: The transparency level of the polygon.

	from shapely.geometry import Point, Polygon
	from shapely.geometry import Point

	from seacharts.core import DataParser, paths
	from seacharts.core import DataParser

Conversation

meeqn commented Nov 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants