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

Difficuty accessing ECCO file #179

Open
waywardpidgeon opened this issue Sep 11, 2024 · 47 comments
Open

Difficuty accessing ECCO file #179

waywardpidgeon opened this issue Sep 11, 2024 · 47 comments

Comments

@waywardpidgeon
Copy link

On trying the example generate_surface_fluxes.jl at the line

mask=ecco_mask()
I get the error (in this minimal example)
run(wget --http-user=kevin_broughan --http-passwd=Nasa44earth-data 'https://ecco.jpl.nasa.gov/drive/files/ECCO2/cube92_latlon_quart_90S90N/daily/THETA\THETA.1440x720x50.19930101.nc')
ERROR: IOError: could not spawn wget --http-user=kevin_broughan --http-passwd=Nasa44earth-data 'https://ecco.jpl.nasa.gov/drive/files/ECCO2/cube92_latlon_quart_90S90N/daily/THETA\THETA.1440x720x50.19930101.nc': no such file or directory (ENOENT)
However, when I login to Nasa earth data and navigate to the file the file its in place at 74.5Mb.

I tried running Julia as su with no change in other problematic examples.

Thanks for looking at this -- Kevin

@glwagner
Copy link
Member

Hi @waywardpidgeon, apologies for the sparse documentation... we are working on it. This page (which is on a PR) explains how to set up your ECCO_USERNAME and ECCO_PASSWORD environment variables:

https://github.com/CliMA/ClimaOcean.jl/tree/glw/clean-up-ECCO/src/DataWrangling/ECCO

Note that the username and password are not your login, but rather the "programmatic API" credentials which are generated by Earthdata.

PS you may want to change your login password now, because it is visible here.

@waywardpidgeon
Copy link
Author

Thanks. I've changed the login password and set up the API creds. However, I'm running under Windows (10 and 11) in the Windows power shell, and this may be the problem with wget (and most other windows commands) needing special treatment from a brief perusal of the web.

wgetERROR.txt

However, I checked the file argument for C:\Users\kab.julia\scratchspaces\0376089a-ecfe-4b0e-a64f-9c555d74d754\Bathymetry
and found that this directory exists but is empty. I have attached the full error report from Julia 1.10.5.

Cheers - Kevin

@waywardpidgeon
Copy link
Author

Also, might we use HTTP.jl instead of run and wget to access and download these sorts of files? See

https://stackoverflow.com/questions/61862089/how-to-use-wget-with-julia

Cheers - Kevin

@glwagner
Copy link
Member

Ok ok

Can you confirm this is the error? If you run

ecco_mask()

on Windows, you'll get the error

run(`wget --http-user=kevin_broughan --http-passwd=Nasa44earth-data 'https://ecco.jpl.nasa.gov/drive/files/ECCO2/cube92_latlon_quart_90S90N/daily/THETA\THETA.1440x720x50.19930101.nc'`)
ERROR: IOError: could not spawn wget --http-user=kevin_broughan --http-passwd=Nasa44earth-data 'https://ecco.jpl.nasa.gov/drive/files/ECCO2/cube92_latlon_quart_90S90N/daily/THETA\THETA.1440x720x50.19930101.nc': no such file or directory (ENOENT)

I think this is coming from here:

cmd = `wget --http-user=$(username) --http-passwd=$(password) $(fileurl)`
run(cmd)

@waywardpidgeon you can confirm if you paste more of the error.

@simone-silvestri why don't we use Base.download here?

@glwagner
Copy link
Member

Hmm it also looks like we apparently would like to test this on Windows, but are not right now,

TEST_GROUP: "downloading"
ECCO_USERNAME: ${{ secrets.ECCO_USERNAME }} # To download ECCO data from the podaac website
ECCO_PASSWORD: ${{ secrets.ECCO_PASSWORD }} # To download ECCO data from the podaac website
- uses: julia-actions/julia-processcoverage@v1

this only tests JRA55

https://github.com/CliMA/ClimaOcean.jl/blob/main/test/test_downloading.jl

@waywardpidgeon
Copy link
Author

Yes, that looks v promising. HTTP.jl and maybe other options I think call wget. The doc julia-1.10.5.pdf on p985 indicates Downloads.download is a currently supported function, with Downloads.jl included in the standard library (not Base).

I've tested this on an HTML file from my website kevinbroughan.nz under Windows 10 and it works ok.

I'll try Greg's test next but note some of the JRA55 requests, e.g. in generate_atmos_dataset.jl, I needed to replace
time_indices = 1:1 by time_indices = Colon()
Given this change, the downloads for JRA55 files appeared to work without error. I'll report on this change once the more difficult wget issue is sorted.
Cheers - Kevin

@waywardpidgeon
Copy link
Author

Hmm it also looks like we apparently would like to test this on Windows, but are not right now,

TEST_GROUP: "downloading"
ECCO_USERNAME: ${{ secrets.ECCO_USERNAME }} # To download ECCO data from the podaac website
ECCO_PASSWORD: ${{ secrets.ECCO_PASSWORD }} # To download ECCO data from the podaac website
- uses: julia-actions/julia-processcoverage@v1

this only tests JRA55

https://github.com/CliMA/ClimaOcean.jl/blob/main/test/test_downloading.jl

I ran this code and here is the output:
@testset "Availability of JRA55 data" begin
@info "Testing that we can download all the JRA55 data..."
for name in ClimaOcean.DataWrangling.JRA55.JRA55_variable_names
fts = ClimaOcean.JRA55.JRA55_field_time_series(name; time_indices=2:3)
end
end
[ Info: Testing that we can download all the JRA55 data...
Test Summary: | Time
Availability of JRA55 data | None 6m31.4s
Test.DefaultTestSet("Availability of JRA55 data", Any[], 0, false, false, true, 1.726181921246e9, 1.726182312606e9, false, "REPL[15]")

Does this mean no JRA55 data is available to my process ?

Thanks - Kevin

@waywardpidgeon
Copy link
Author

I looked again at one of the original errors and observed the url had single, not double quotes. Also before the file name there was a backslash not forward, another gremlin. Given these two url fixes I tried Downloads.download(url) and it downloaded a file of over 470Mb into a temporary file with name returned by the function. So I assume it works for ECCO files.
There was no request for username or password - these are now in my user environment, so it might have picked them up automatically.
Cheers - Kevin
PS I'm not confident enough yet to make a pull request and attempt changes.

Sorry about Windows, but there must be many users who have it so I'm sticking with this OS.

@glwagner
Copy link
Member

No need to apologies!

I think a more direct test will be to paste this code in your REPL

using ClimaOcean
using Oceananigans
JRA55_temperature = ClimaOcean.JRA55.JRA55_field_time_series(:temperature; time_indices=2:3)

Then you can try to play around with, inspect, or plot JRA55_temperature to see if indeed JRA55 data was downloaded.

PS: to make the errors easier to read when you paste them here, try surrounding them in triple backticks (```). This is how your format "code blocks" in markdown (also googling "code blocks in markdown" may help)

@simone-silvestri
Copy link
Collaborator

Sorry for the late reply. I forgot that wget does not work on windows machines. Good catch. If you make a PR to correct this oversight it would be great.

@waywardpidgeon
Copy link
Author

I think an expert should fix this with a PR, not me yet.

Here is my workaround for wget producing an error in Windows: I used Downloads.download and the ECCO data for the example generated_bathymetry.jl ended up in my Downloads folder. I moved it into the scratchspaces subdirectory of .julia (the file was about 0.5 Gbytes), the subdirectory possibly having been created by the generate_atmos_dataset.jl example. I then ran the example generated_bathymetry.jl again and it picked up the file ("existing file") without any further difficulty (other than the warning "resolution->size" for the Figure Makie function).

@simone-silvestri
Copy link
Collaborator

I do not have immediate access to a windows machine. If I open a PR to fix this, can you check that downloading works? We should probably add Windows tests in the CI to catch these architecture-dependent bugs

@waywardpidgeon
Copy link
Author

OK I will test.

Windows tests would be a good idea. Its nice to be alert to the problem - I think I have struck it in the past and not known it!

I am working on the generate_surface_fluxes.jl example, having downloaded the ECCO2 data directly from the JPL data store. Is there a path that I can find or establish on my machine where I can put this data and the program (ecco_mask() and its calls) will find it?
Thanks - Kevin

@glwagner
Copy link
Member

@simone-silvestri we do have access to windows via github actions. We need to add a test there.

@waywardpidgeon
Copy link
Author

Another case: in the example inspect_ecco_data.jl at line 20 we see a reference to the "default location' from which ecco_field is expected to access the data if it exists (see the doc for ecco_field in ECCO.jl item (1)). I want to place the data (which I have) at this location, but I don't know what it is? What is it and how in general can I find it?
Thanks - Kevin

@glwagner
Copy link
Member

Can you put hyperlinks to the source code here?

@waywardpidgeon
Copy link
Author

@glwagner
Copy link
Member

glwagner commented Sep 18, 2024

Ah yes, it's also possible to generate a link to the line itself (which is nice because GitHub will display that line directly in the comment, convenient because we don't have to click on it). A link to the file is helpful too though, thank you.

To generate a link to a line look for the three dots that appear when hovering your mouse over the line numbers on the left side of a file being viewed on GitHub.

@glwagner
Copy link
Member

@waywardpidgeon you mean these lines?

# The function `ecco_field` provided by `ClimaOcean.DataWrangling.ECCO` will automatically
# download ECCO data if it doesn't already exist at the default location.
T = ECCO.ecco_field(:temperature)
S = ECCO.ecco_field(:salinity)

@glwagner
Copy link
Member

Ok ok I understand the question.

It looks like you can provide a filename:

function ecco_field(metadata::ECCOMetadata;
architecture = CPU(),
horizontal_halo = (3, 3),
filename = metadata_filename(metadata))
shortname = short_name(metadata)
download_dataset!(metadata)
ds = Dataset(filename)

But it also seems that it will still try to download the dataset even if the filename is provided.

This seems really buggy and fragile but maybe there is an underlying purpose to this design that @simone-silvestri can explain.

Sorry to say @waywardpidgeon that this stuff is in flux a little bit and we need to clean it up for sure. From your experience I understand that:

  1. We need to test auto-downloading on windows
  2. We should allow users to pass data paths, and allow downloading to be skipped if the data exists at that custom path.

What else?

@simone-silvestri
Copy link
Collaborator

simone-silvestri commented Sep 18, 2024

Yeahm the path cannot be specified, that in theory, was one of the objectives of #180.
download_dataset! checks if the file exists, and if it does not, it dowloads the file

if !isfile(filename)
isnothing(username) && throw(ArgumentError("Could not find the username for $(url). Please provide a username in the ECCO_USERNAME environment variable."))
isnothing(password) && throw(ArgumentError("Could not find the username for $(url). Please provide a password in the ECCO_PASSWORD environment variable."))
# Version specific download file url
if data.version isa ECCO2Monthly || data.version isa ECCO2Daily
.
So we should change the download command to downloads and add a path for the ecco files.
In theory, in #180 we provide a directory to the ECCOMetadata type that specifies the path of the files. That will help us with generalizing the data-wrangling module.

@simone-silvestri
Copy link
Collaborator

simone-silvestri commented Sep 18, 2024

we can also probably remove the filename keyword argument. The user should not be able to choose the filename, I think.

@glwagner
Copy link
Member

I think we should polish this implementation first and then we can add new features, eg supporting manually-downloaded files for example.

But yeah in general it is good if everyone uses the same filename, it will help communication.

@waywardpidgeon
Copy link
Author

OK, I'll stop with this work now for a while. Plenty else to do. Here is where I'm at. The function urlread works under Windows 11

using HTTP
using Downloads: download

function urlread(url, un, output)
pw =HTTP.escapeuri(ENV["ECCO_PASSWORD"])
cred_url="https://$un:$pw@" * replace(url, r"^.*://"=>"")
download(cred_url, output)
end
url="http://ecco.jpl.nasa.gov/drive/files/ECCO2/cube92_latlon_quart_90S90N/daily/THETA/THETA.1440x720x50.19930101.nc"
un="your_ecco_name"
output= "path_including_file"

The download function did not pick up the username or password from the environment and It did not appear to read these with .netrc or _netrc, but building in both API credentials into the URL worked (picked up the method from SpaceLiDAR.jl). This function returned a path and created the file but seemed to need existing subdirectories to actually work.

Other tasks: getting the output into Julia, including the Sys.iswindows() conditional, creating the canonical subdirectories of .julia/scratchspaces, doing a good check of other wget occurrences, etc.

Thanks for the advice. I try and come more up to speed with git next time - Kevin
PS my git password should change to [email protected] soon.

@gaelforget
Copy link

ping @ifenty , @menemenlis @hongandyan , @owang01

@hongandyan
Copy link

hongandyan commented Oct 12, 2024 via email

@glwagner
Copy link
Member

Ah interesting. @hongandyan how exactly are you suggesting that we use llcreader and fsspec?

There are two ways that we use ECCO data at the moment:

  1. As a source of initial conditions for ocean simulations from regional to global
  2. As "restoring to climatology" on the boundaries of a regional configuration

I believe we want to support predownloading all of the data required for a simulation. Though it is interesting (and I hadn't considered it before) to support the optimization whereby data is downloaded while the simulation is running. This wouldn't be available to all users (because some users do not have internet on compute nodes).

If there is an advantage to fsspec and llcreader it'd be great to leverage that. It does introduce the heavy dependency of PyCall so we want to be sure it is well-motivated. I'm not 100% sure what it would add on top of what we already have.

I think the central difficulty of this current issue is how to download data to windows machines. I think with wget we are able to use Programmatic API credentials which is pretty easy. This procedure is described in the README here:

https://github.com/CliMA/ClimaOcean.jl/tree/main/src/DataWrangling/ECCO

But on windows we don't have wget so we need another solution.

@owang01
Copy link

owang01 commented Oct 16, 2024

curl is per-installed on Windows 10 and later machines, so it can be an alternative solution.

@waywardpidgeon
Copy link
Author

I tried the example given in the LibCURL.jl docs on the URL https://kevinbroughan.nz , ie. given in

https://github.com/JuliaWeb/LibCURL.jl/blob/master/README.md

Not knowing what "per-installed" meant I tried to add the package with lots of errors from precompilation. However
the functions in the example. The line # execute the query returned https://kevinbroughan.nz/index.html to the stout or screen.
The other lines returned integers in the main: In particular res was bound to 0x0...0 and http code to an 1 element array with value http_code[1]-> 200.

See the example from https://github.com/JuliaWeb/LibCURL.jl/blob/master/README.md is annotated below:

using LibCURL

# init a curl handle
curl = curl_easy_init()

# set the URL and request to follow redirects
curl_easy_setopt(curl, CURLOPT_URL, "http://kevinbroughan.nz")
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1)


# setup the callback function to recv data
function curl_write_cb(curlbuf::Ptr{Cvoid}, s::Csize_t, n::Csize_t, p_ctxt::Ptr{Cvoid})
    sz = s * n
    data = Array{UInt8}(undef, sz)

    ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt64), data, curlbuf, sz)
    println("recd: ", String(data))

    sz::Csize_t
end

c_curl_write_cb = @cfunction(curl_write_cb, Csize_t, (Ptr{Cvoid}, Csize_t, Csize_t, Ptr{Cvoid}))
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, c_curl_write_cb)


# execute the query
res = curl_easy_perform(curl) # <<<<<======== HTML for index.html went to stdout (screen)

println("curl url exec response : ", res) 
# res was not bound to the HTML

# retrieve HTTP code
http_code = Array{Clong}(undef, 1)
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, http_code)
println("httpcode : ", http_code)
# http code was not retrieved apparently


# release handle
curl_easy_cleanup(curl)

I didn't check web pages for which I need username/password credentials.

julia> versioninfo()
Julia Version 1.10.5
Commit 6f3fdf7b36 (2024-08-27 14:19 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: 24 × 13th Gen Intel(R) Core(TM) i7-13700F
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, goldmont)
Threads: 1 default, 0 interactive, 1 GC (on 24 virtual cores)

I would think using Downloads.download with output to a file, which is wrangled as needed and stored in a good place, and then read back whenever needed, would be a viable solution for most purposes? I hope someone is fixing these Julia/Windows deficiencies for ClimaOcean?

For example, the ECCO data has to have credentials updated every few months and its tedious to go back to Nasa/ECCO each time (in spite of the good docs in ClimaOcean.jl). CliMA/ClimaArtifacts (no .jl) works ok (with some variations due using a subpackage structure). Thus, putting the data into a local file or global artifact repo seems positive - the use of data would seldom be one-off.

@glwagner
Copy link
Member

glwagner commented Nov 6, 2024

Thus, putting the data into a local file or global artifact repo seems positive - the use of data would seldom be one-off.

Using repos isn't feasible in general. For example JRA55 data is something like 1.7T.

For example, the ECCO data has to have credentials updated every few months and its tedious to go back to Nasa/ECCO each time (in spite of the good docs in ClimaOcean.jl).

I'm not sure there is any way to avoid credentials for big expensive downloads. We would also need to set that up for GLORYS and ERA5, right? And future datasets, presumably...

I didn't check web pages for which I need username/password credentials.

This is the key thing we need, because otherwise Downloads.download works fine.

@waywardpidgeon if you can indeed come up with a comprehensive strategy that ensures we don't need credentials for any data we want to use, I think that is very interesting and we should pursue it! But the thinking I have devoted to this has not come up with any alternative solution unfortunately.

Notice that local files are certainly currently supported (one only needs to drop the file in the location that it would be downloaded), but probably a bit more effort needs to be expended to make this easier. @waywardpidgeon this could be a good area for a contribution!

@glwagner
Copy link
Member

Hi @waywardpidgeon just wanted to point out that if you want to supply a custom directory to the location of the ECCO data, you can do that via ECCOMetadata:

help?> ClimaOcean.DataWrangling.ECCO.ECCOMetadata
  struct ECCOMetadata{D, V}

  Metadata information for an ECCO dataset:

    •  name: The name of the dataset.

    •  dates: The dates of the dataset, in an AbstractCFDateTime format.

    •  version: The version of the dataset, could be ECCO2Monthly, ECCO2Daily, or ECCO4Monthly.

    •  dir: The directory where the dataset is stored.

  ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  ECCOMetadata(name::Symbol;
               dates = DateTimeProlepticGregorian(1993, 1, 1),
               version = ECCO4Monthly(),
               dir = download_ECCO_cache)

  Construct an ECCOMetadata object with the specified parameters.

  Arguments
  ≡≡≡≡≡≡≡≡≡

    •  name::Symbol: The name of the metadata.

  Keyword Arguments
  ≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡

    •  dates: The date(s) of the metadata. Note this can either be a single date, representing a snapshot, or a range of dates, representing a
       time-series. Default: DateTimeProlepticGregorian(1993, 1, 1).

    •  version: The data version. Supported versions are ECCO2Monthly(), ECCO2Daily(), or ECCO4Monthly().

    •  dir: The directory of the data file. Default: download_ECCO_cache.

I'm not sure if this is what you are doing. But if so please give the dir keyword a try and let us know if it is what you are looking for.

I also think possibly using curl or some other would be nice if it is supported on windows, we just need to understand if it works with credentials.

Any help is greatly appreciated because there is quite a bit to do at this early stage to get this package groomed and well-oiled!

@waywardpidgeon
Copy link
Author

waywardpidgeon commented Nov 11, 2024 via email

@glwagner
Copy link
Member

Indeed, that's what I have been doing these past days, i.e. finding each required path and filename and downloading into the appropriate ClimaOcean scratchspace.

Did you try downloading to the directory of your choice and using the dir keyword in ECCOMetadata?

The "downloading the data apriori" strategy requires a fix to the "joinpath" function, which has a flaw under Windows, which I have fixed using simply join([..,..,..]).

joinpath is defined in Base:

help?> joinpath
search: joinpath

  joinpath(parts::AbstractString...) -> String
  joinpath(parts::Vector{AbstractString}) -> String
  joinpath(parts::Tuple{AbstractString}) -> String

  Join path components into a full path. If some argument is an absolute path
  or (on Windows) has a drive specification that doesn't match the drive
  computed for the join of the preceding paths, then prior components are
  dropped.

  Note on Windows since there is a current directory for each drive,
  joinpath("c:", "foo") represents a path relative to the current directory on
  drive "c:" so this is equal to "c:foo", not "c:\foo". Furthermore, joinpath
  treats this as a non-absolute path and ignores the drive letter casing,
  hence joinpath("C:\A","c:b") = "C:\A\b".

  Examples
  ≡≡≡≡≡≡≡≡

  julia> joinpath("/home/myuser", "example.jl")
  "/home/myuser/example.jl"

  julia> joinpath(["/home/myuser", "example.jl"])
  "/home/myuser/example.jl"

julia> @which joinpath
Base.Filesystem

I would be surprised that it has an error though. It could make sense to ask a question to the helpdesk channel on the Julia slack before opening an issue on the julialang github.

With a small fix in the set! call (change date to dates=date in both S and T calls)

If you can point me to the problematic script I would love to fix it. Or you can submit a PR. I looked at near_global_ocean_simulation.jl but it looks in order there:

set!(ocean.model, T=ECCOMetadata(:temperature; dates=date), S=ECCOMetadata(:salinity; dates=date))

@waywardpidgeon
Copy link
Author

Here is an highlighted pdf file with the small number of fixes for the initial ClimaOcean.jl page doc.

Following the preliminary downloading to the ClimaOcean scratchspace subdirectories of the data files needed to run the preliminary example in the docs, and the minor fixes to the set! call, the example completed sucessfully in less than 9 hours wall time on a Nvidia GPU workstation. Great!

Greg13nov24.pdf

The result is not as good as what we see in the docs. A zoomed and Colorbar form of the output is attached. I'd like to see how to show the same degree of turbulence which is in the docs following "which produces", even if the compute time went up by say a factor of 10?

ClimaOceanDocsEx.pdf

@waywardpidgeon
Copy link
Author

waywardpidgeon commented Nov 13, 2024 via email

@glwagner
Copy link
Member

Here is an highlighted pdf file with the small number of fixes for the initial ClimaOcean.jl page doc.

Following the preliminary downloading to the ClimaOcean scratchspace subdirectories of the data files needed to run the preliminary example in the docs, and the minor fixes to the set! call, the example completed sucessfully in less than 9 hours wall time on a Nvidia GPU workstation. Great!

Greg13nov24.pdf

The result is not as good as what we see in the docs. A zoomed and Colorbar form of the output is attached. I'd like to see how to show the same degree of turbulence which is in the docs following "which produces", even if the compute time went up by say a factor of 10?

ClimaOceanDocsEx.pdf

Possibly some bugs have crept into ClimaOcean on account of #237...

I can try to run again and see what I get

@waywardpidgeon
Copy link
Author

I have started working on a proposed fix for the Windows download issue, but having difficulty with the eding/workflow. I am using Notepad++ and incl

@waywardpidgeon
Copy link
Author

I notice the development version of Bathymetry.jl as of 21/11/24 has had the wget call removed. Is someone else working on the Windows fix to ClimaOcean files?

I get errors as before with the generate_bathymetry.jl example after I have removed the scratchspace bathymetry data.

I have been adding credentials to the url for the download call to get the bathymetry.

@waywardpidgeon
Copy link
Author

Oh, now I recall, it is not the development version, but the rev="main" one I should be editing!

@waywardpidgeon
Copy link
Author

I have made some edits to Bathymetry.jl and ECCO_metadata.jl to provide support for Windows data downloads. There may be other files where this is needed for ClimaOcean?
I needed to include src/DataWrangling/DataWrangling.jl and src/DataWranglink/ECCO/ECCO.jl explicitly, as well as the files I was working on.
I commented out @root which I could not find code for and which was needed, and will need to be restored for message passing I think.

I don't have ready access to Linux, so have not tested the code in that environment.

Here are the edited files
kevin28nov24.zip
(sorry, no .tar or .jl !)

@waywardpidgeon
Copy link
Author

Please when picking over the code I sent for Bathymetry.jl remove the closing braket at the end of line 132: @info "Data is in filepath = $(filepath) and is next being manipulated a little ....")

└ ── Expected end

Thanks

@simone-silvestri
Copy link
Collaborator

Thats for the work @waywardpidgeon. Do you want me to open a pull request with the improvements or do you want to open a pull request yourself?

@waywardpidgeon
Copy link
Author

waywardpidgeon commented Nov 28, 2024 via email

@glwagner
Copy link
Member

Please make the pull request yourself. Thanks.

________________________________ From: Simone Silvestri @.> Sent: Friday, November 29, 2024 1:03:46 AM To: CliMA/ClimaOcean.jl @.> Cc: Kevin Broughan @.>; Mention @.> Subject: Re: [CliMA/ClimaOcean.jl] Difficuty accessing ECCO file (Issue #179) Thats for the work @waywardpidgeonhttps://github.com/waywardpidgeon. Do you want me to open a pull request with the improvements or do you want to open a pull request yourself? — Reply to this email directly, view it on GitHub<#179 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AUPNYEXVHIR24MN5GARFRZL2C4BCFAVCNFSM6AAAAABN777WBKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBVHE2TSOBUGA. You are receiving this because you were mentioned.Message ID: @.***>

It's no problem, but if you make the PRs then you contribution to this package and ocean modeling will be encoded into the github metrics! Just an FYI, it can be nice to get credit for your work.

@simone-silvestri
Copy link
Collaborator

This Issue should be fixed, can you confirm @waywardpidgeon?

@waywardpidgeon
Copy link
Author

waywardpidgeon commented Dec 16, 2024 via email

@glwagner
Copy link
Member

glwagner commented Dec 17, 2024

These errors include a circular dependency warning and failure to precompile CairoMakie, but might all be a fault of my installation.

This is a Julia issue, but you should be able to press forward despite getting the warnings/errors

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 a pull request may close this issue.

6 participants