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

WIP: Get fieldtypes from defaults #138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP: Get fieldtypes from defaults #138

wants to merge 2 commits into from

Conversation

mauro3
Copy link
Owner

@mauro3 mauro3 commented Feb 25, 2021

Resolves #131

julia> @with_kw struct MT5{I}                                                                                                                                                                   
           r::typeof=5                                                                                                                                                                          
           a::I                                                                                                                                                                                 
       end                                                                                                                                                                                      
MT5                                                                                                                                                                                             
                                                                                                                                                                                                
julia> MT5(5.5, 1)                                                                                                                                                                              
ERROR: InexactError: Int64(5.5)                                                                                                                                                                 
Stacktrace:                                                                                                                                                                                     
 [1] Int64 at ./float.jl:710 [inlined]                                                                                                                                                          
 [2] convert at ./number.jl:7 [inlined]                                                                                                                                                         
 [3] MT5 at /home/mauro/julia/dot-julia-dev/Parameters/src/Parameters.jl:489 [inlined]                                                                                                          
 [4] MT5(::Float64, ::Int64) at /home/mauro/julia/dot-julia-dev/Parameters/src/Parameters.jl:512                                                                                                
 [5] top-level scope at REPL[3]:1                                                                                                                                                               
                                                                                                                                                                                                
julia> MT5(5, 1)                                                                                                                                                                                
MT5{Int64}                                                                                                                                                                                      
  r: Int64 5                                                                                                                                                                                    
  a: Int64 1                                                                                                                                                                                    

and

julia> @with_kw struct MT2 @deftype typeof
           r=5
           a
       end
MT2

julia> MT2(4.5,1)
ERROR: InexactError: Int64(4.5)
Stacktrace:
 [1] Int64 at ./float.jl:710 [inlined]
 [2] convert at ./number.jl:7 [inlined]
 [3] MT2(::Float64, ::Int64) at /home/mauro/julia/dot-julia-dev/Parameters/src/Parameters.jl:489
 [4] top-level scope at REPL[3]:1

julia> MT2(4,1)
MT2
  r: Int64 4
  a: Int64 1

@codecov
Copy link

codecov bot commented Feb 25, 2021

Codecov Report

Merging #138 (ecbf8df) into master (cf8fec6) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #138   +/-   ##
=======================================
  Coverage   93.66%   93.66%           
=======================================
  Files           1        1           
  Lines         284      284           
=======================================
  Hits          266      266           
  Misses         18       18           
Impacted Files Coverage Δ
src/Parameters.jl 93.66% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf8fec6...b88a51f. Read the comment docs.

@bgroenks96
Copy link

So it isn't working for me, but I think it's because Julia is somehow not using the updated version (error line numbers don't make sense).

What is the general best practice for doing this? I initially did develop ... on my fork and then checked out the branch. But even after doing activate . it didn't seem to recompile Parameters.jl. Same thing happens when I do add <url>#m3/typeof. So I'm a bit at a loss how to get Julia to recompile and/or use the right version without deleting my entire .julia folder and starting from scratch...

@mauro3
Copy link
Owner Author

mauro3 commented Feb 28, 2021

You first create a dev of Parameters. E.g. start the repl and do ], dev Parameters. That will create ~/.julia/dev/Parameters, a git repo. quit the repl, cd to ~/.julia/dev/Parameters, do git checkout m3/typeof. Starting as julia --project should get your repl started with Parameters as active project. Probably start with ] and test to see whether test currently pass. Then start edidting. Once you want to commit, maybe make a new branch git checkout -b bt/typeof-addons, git commit -am "...". The add your fork as remote git remote add bgroenks96 ...url..., then git push bgroenks96. The open the PR.

Does that help?

@bgroenks96
Copy link

Hi @mauro3 . Sorry for the long delay on this... I got caught up in other things.

I am of course familiar with git and everything, that's not so much the issue. The issue was just that julia didn't seem to recompile Parameters, or was for some reason using the wrong version, I still don't really know. I set everything up on a separate machine with a clean environment and now it works. I am working on tests now!

@bgroenks96
Copy link

@mauro3 So far so good except for one somewhat contrived issue with @deftype and field-dependent default values:

@with_kw struct I131c @deftype typeof
    a = 5
    b = 1.0
    c = a + b
end

throws error ERROR: LoadError: UndefVarError: a not defined.

Like I said, this is somewhat contrived, and one workaround is to just explicitly declare the type ::Float64 on c. We could just document this and open a new issue for later, unless you have a quick fix in mind...?

@mauro3
Copy link
Owner Author

mauro3 commented Mar 29, 2021

Ok, that is a good catch. It might be a bit hard to support this, I'll see.

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.

Automatically infer type based on default
2 participants