-
Notifications
You must be signed in to change notification settings - Fork 12
Fix/qgis plugin fixes #243
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
base: master
Are you sure you want to change the base?
Conversation
Makes passing handler from qgis to python possible
There was a problem hiding this 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 pull request implements fixes and improvements for QGIS plugin integration and general code quality enhancements. The changes focus on improving configurability of column names, adding defensive programming practices, and implementing callable interfaces for key classes.
Changes:
- Enhanced column name configurability across sorter classes to support different data schemas
- Added
__call__methods to Sampler, Sorter, and ThicknessCalculator for callable interface pattern - Improved handling of empty data and edge cases in thickness calculations and sorting algorithms
- Fixed type hints and added explicit import paths
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/project/test_thickness_calculations.py | Updated import to use explicit module path for Project class |
| map2loop/utils/utility_functions.py | Enhanced type hints to accept Union types for numeric parameters and added GeometryCollection return type |
| map2loop/topology.py | Added configurable id_field parameter for fault identification |
| map2loop/thickness_calculator.py | Improved location_tracking handling for empty data, added LineString import, fixed indentation, added call method, and removed duplicate UNITNAME column before spatial join |
| map2loop/sorter.py | Added unit_name_column, unitname1_column, and unitname2_column configurability, imported networkx at module level, enhanced length column handling, and added call method |
| map2loop/sampler.py | Added call method for callable interface |
| map2loop/project.py | Added automatic configuration of min_age_column and max_age_column for sorters |
| map2loop/logging.py | Added new setLogging function for flexible logging configuration |
Comments suppressed due to low confidence (2)
map2loop/topology.py:50
- The new id_field parameter is missing documentation in the docstring. The Args section should be updated to include this parameter and explain its purpose.
"""
The initialiser for the map2model wrapper
Args:
map_data (MapData):
The project map data structure to reference
verbose_level (VerboseLevel, optional):
How much console output is sent. Defaults to VerboseLevel.ALL.
"""
map2loop/logging.py:49
- This assignment to 'logger' is unnecessary as it is redefined before this value is used.
logger = getLogger(__name__)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
map2loop/sorter.py
Outdated
| required_arguments = ['min_age_column','max_age_column'] | ||
| def __init__(self,*, min_age_column:Optional[str]='minAge', max_age_column:Optional[str]='maxAge'): | ||
| required_arguments = ['min_age_column','max_age_column','unit_name_column'] | ||
| def __init__(self,*, unit_name_column:Optional[str]='name',min_age_column:Optional[str]='minAge', max_age_column:Optional[str]='maxAge'): |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type hint parameter should have spaces around the equals sign for consistency with Python style guidelines. Should be 'unit_name_column: Optional[str] = 'name'' instead of 'unit_name_column:Optional[str]='name''.
| def __init__(self,*, unit_name_column:Optional[str]='name',min_age_column:Optional[str]='minAge', max_age_column:Optional[str]='maxAge'): | |
| def __init__( | |
| self, | |
| *, | |
| unit_name_column: Optional[str] = 'name', | |
| min_age_column: Optional[str] = 'minAge', | |
| max_age_column: Optional[str] = 'maxAge', | |
| ): |
map2loop/sorter.py
Outdated
| self, | ||
| *, | ||
| contacts: Optional[geopandas.GeoDataFrame] = None, | ||
| unit_name_column:Optional[str]='name', |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type hint parameter should have spaces around the equals sign for consistency with Python style guidelines. Should be 'unit_name_column: Optional[str] = 'name'' instead of 'unit_name_column:Optional[str]='name''.
map2loop/utils/utility_functions.py
Outdated
| start_point: shapely.geometry.Point, azimuth_deg: float, distance: int, bbox: pandas.DataFrame | ||
| ) -> shapely.geometry.LineString: | ||
| start_point: shapely.geometry.Point, azimuth_deg: Union[float,int], distance: Union[float,int], bbox: pandas.DataFrame | ||
| ) -> Union[shapely.geometry.LineString,shapely.geometry.GeometryCollection]: |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type hints should have spaces after commas for consistency with Python style guidelines. Should be 'Union[shapely.geometry.LineString, shapely.geometry.GeometryCollection]' instead of 'Union[shapely.geometry.LineString,shapely.geometry.GeometryCollection]'.
| ) -> Union[shapely.geometry.LineString,shapely.geometry.GeometryCollection]: | |
| ) -> Union[shapely.geometry.LineString, shapely.geometry.GeometryCollection]: |
map2loop/sorter.py
Outdated
| *, | ||
| contacts: Optional[geopandas.GeoDataFrame] = None, | ||
| unit_name_column:Optional[str]='name', | ||
| unitname1_column:Optional[str]='UNITNAME_1', |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type hint parameter should have spaces around the equals sign for consistency with Python style guidelines. Should be 'unitname1_column: Optional[str] = 'UNITNAME_1'' instead of 'unitname1_column:Optional[str]='UNITNAME_1''.
map2loop/sorter.py
Outdated
| required_arguments = ['min_age_column','max_age_column'] | ||
| def __init__(self,*, min_age_column:Optional[str]='minAge', max_age_column:Optional[str]='maxAge'): | ||
| required_arguments = ['min_age_column','max_age_column','unit_name_column'] | ||
| def __init__(self,*, unit_name_column:Optional[str]='name',min_age_column:Optional[str]='minAge', max_age_column:Optional[str]='maxAge'): |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameters should have spaces after commas and around equals signs for consistency with Python style guidelines. Should be 'unit_name_column: Optional[str] = 'name', min_age_column: Optional[str] = 'minAge', max_age_column: Optional[str] = 'maxAge''.
| def __init__(self,*, unit_name_column:Optional[str]='name',min_age_column:Optional[str]='minAge', max_age_column:Optional[str]='maxAge'): | |
| def __init__(self, *, unit_name_column: Optional[str] = 'name', min_age_column: Optional[str] = 'minAge', max_age_column: Optional[str] = 'maxAge'): |
| df = pandas.DataFrame(0, index=unit_names, columns=unit_names) | ||
| for younger, older in ordered_unit_observations: | ||
| df.loc[younger, older] += 1 | ||
| print(df, df.max()) |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug print statement should be removed before merging to production code.
map2loop/utils/utility_functions.py
Outdated
| start_point: shapely.geometry.Point, azimuth_deg: Union[float,int], distance: Union[float,int], bbox: pandas.DataFrame | ||
| ) -> Union[shapely.geometry.LineString,shapely.geometry.GeometryCollection]: |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type hints should have spaces after commas for consistency with Python style guidelines. Should be 'Union[float, int]' instead of 'Union[float,int]'.
| start_point: shapely.geometry.Point, azimuth_deg: Union[float,int], distance: Union[float,int], bbox: pandas.DataFrame | |
| ) -> Union[shapely.geometry.LineString,shapely.geometry.GeometryCollection]: | |
| start_point: shapely.geometry.Point, azimuth_deg: Union[float, int], distance: Union[float, int], bbox: pandas.DataFrame | |
| ) -> Union[shapely.geometry.LineString, shapely.geometry.GeometryCollection]: |
map2loop/sorter.py
Outdated
| def __init__( | ||
| self, | ||
| *, | ||
| unit_name_column:Optional[str]='name', |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type hint parameter should have spaces around the equals sign for consistency with Python style guidelines. Should be 'unit_name_column: Optional[str] = 'name'' instead of 'unit_name_column:Optional[str]='name''.
| unit_name_column:Optional[str]='name', | |
| unit_name_column: Optional[str] = 'name', |
Description
📝 Thanks for contributing to map2loop!
Please describe the issue that this pull request addresses and summarize the changes you are implementing.
Include relevant motivation and context, if appropriate.
List any new dependencies that are required for this change.
Fixes #(issue)
Type of change
How Has This Been Tested?
Please describe any tests that you ran to verify your changes.
Provide branch name so we can reproduce.
Checklist:
Checklist continued (if PR includes changes to documentation)