forked from MassimoCimmino/pygfunction
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Potential API for PyGFunction #4
Open
vtnate
wants to merge
40
commits into
master
Choose a base branch
from
api
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
5294981
example api showing how to pass in config and return gfunction
vtnate 21ccaa8
test demonstrating use of api
vtnate f6241ab
fix typing and allow options dict
vtnate 97a7068
add todo in api test
vtnate dce7030
some cleanups
mitchute a4ec330
fix type hinting that I broke
mitchute 2c3cf40
incorporate suggestions
mitchute 1c9d52a
fix typing
mitchute a9a18df
more typing
mitchute 3e0869a
cleanup tests
mitchute b2f5988
Updated API file a bit
Myoldmopar cbf728c
add docstrings and other documenting comments
mitchute ce9d520
combine classes, update docs
mitchute a67ad63
return list
mitchute a4fb763
Initial implementation of Borefield class
MassimoCimmino f52c143
Class method to evaluate g-function
MassimoCimmino 5f86520
Change init to use arrays instead of boreholes
MassimoCimmino 7c1bfc8
Fix broadcasting of tilt and orientation
MassimoCimmino 8320711
Fix borefield.from_file to call class init
MassimoCimmino 5f31f6b
Docstrings
MassimoCimmino 4707959
Remove api module
MassimoCimmino fc78296
Test g-functions
MassimoCimmino f2d4341
Test borefield.from_boreholes
MassimoCimmino 4806d16
Implement bore field creation methods
MassimoCimmino d5374ce
Deprecation warnings for old bore field functions
MassimoCimmino 1a5f0c1
Fix Borefield.from_file
MassimoCimmino 59e0d99
Update examples
MassimoCimmino 6027e42
Boreholes can have negative tilt
MassimoCimmino f999eac
Save field to file
MassimoCimmino 2010311
Fix dense and box fields
MassimoCimmino 4364c06
Update tests
MassimoCimmino 0645d57
Remove blank space
MassimoCimmino 8f5543c
Import Self from typing_extensions
MassimoCimmino c512fc7
Update CHANGELOG.md
MassimoCimmino 5972850
Update requirements
MassimoCimmino e3c92d7
Review changes
MassimoCimmino 34d5543
Re-order methods
MassimoCimmino 033c9ac
Implement Borefield.visualize_field
MassimoCimmino 1084a09
Typing for Borefield.visualize_field
MassimoCimmino a37a8ca
Final review
MassimoCimmino File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,4 @@ numpydoc >= 1.2.0 | |
recommonmark >= 0.6.0 | ||
sphinx >= 4.4.0 | ||
secondarycoolantprops >= 1.1.0 | ||
typing_extensions >= 4.0.1 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@MassimoCimmino thanks, this looks great! One minor point - python 3.8 is past end of life, so perhaps in a separate PR this could be bumped up to a more current version.
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.
Yes. I tend to update Python versions and dependencies on major and minor releases. This is one place where I had forgotten to update on the last version.
A little summary of my implementation if you want to discuss any of the changes:
gFunction
class now acceptsBorefield
objects. That did not require changes in thegFunction
class (outside ofisinstance
checks) since the new class behaves like a list of boreholes for all operations done in pygfunction.Borefield.evaluate_g_function
class method returns an array of g-function values. This was to align with the currentgFunction.evaluate_g_function
method.boreholes.rectangle_field
) to theBorefield
class. They needed to be re-implemented to generate arrays of geometrical parameters instead of lists of boreholes. The functions in theboreholes
module now have a deprecation warning.Something to consider is if the
boreholes.visualize_borefield
should also be moved to theBorefield
class. I remember we had a discussion about the dependency for matplotlib. An option is to include the method and import matplotlib at runtime within the method instead of the top of the module.The PR is much larger than I expected. I will review it and make sure it is compatible with the planned changes for issue MassimoCimmino#210.