Skip to content
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

Add some constructors #654

Merged
merged 6 commits into from
Aug 31, 2022
Merged

Add some constructors #654

merged 6 commits into from
Aug 31, 2022

Conversation

stelmo
Copy link
Collaborator

@stelmo stelmo commented Aug 27, 2022

Useful for model building. Basically moved the existing internal constructors outside so that they can be documented and appear when you look for them at the repl.

@stelmo
Copy link
Collaborator Author

stelmo commented Aug 27, 2022

/format

@github-actions
Copy link
Contributor

✔️ Auto-formatting triggered by this comment succeeded, commited as 55f79ec

github-actions bot pushed a commit that referenced this pull request Aug 27, 2022
triggered by @stelmo on PR #654
@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Merging #654 (6a1c1ca) into develop (7ed54eb) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #654   +/-   ##
========================================
  Coverage    91.05%   91.06%           
========================================
  Files           82       82           
  Lines         2012     2014    +2     
========================================
+ Hits          1832     1834    +2     
  Misses         180      180           
Impacted Files Coverage Δ
src/base/types/Gene.jl 100.00% <100.00%> (ø)
src/base/types/Metabolite.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@stelmo stelmo requested a review from exaexa August 30, 2022 19:21
@stelmo stelmo added the minor Easy fix with low priority, likely just cosmetic label Aug 30, 2022
Copy link
Collaborator

@exaexa exaexa left a comment

Choose a reason for hiding this comment

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

This should be rebased when the DSE PR is merged. (#661)

Also, shouldn't we use Base.@kwdef or some of the pals as we do with SBML? It makes this kind of constructors automagically.

Ref: https://github.com/LCSB-BioCore/SBML.jl/blob/master/src/structs.jl#L413

@stelmo
Copy link
Collaborator Author

stelmo commented Aug 31, 2022

Oh yeah, Base.kwdef will be good to add. Okay, will do once rebased

@stelmo
Copy link
Collaborator Author

stelmo commented Aug 31, 2022

Ah actually, I want the docstrings, not sure if kwdef adds docstrings by default?

@stelmo
Copy link
Collaborator Author

stelmo commented Aug 31, 2022

/format

triggered by @stelmo on PR #654
@github-actions
Copy link
Contributor

✔️ Auto-formatting triggered by this comment succeeded, commited as 6a1c1ca

@stelmo stelmo requested a review from exaexa August 31, 2022 10:34
Copy link
Collaborator

@exaexa exaexa left a comment

Choose a reason for hiding this comment

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

cool

@stelmo stelmo merged commit c8802d3 into develop Aug 31, 2022
@stelmo stelmo deleted the mo-constructors branch August 31, 2022 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Easy fix with low priority, likely just cosmetic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants